laurilehmijoki/jekyll-s3

Example and code for non-CLI usage

adelevie opened this issue · 7 comments

adelevie commented

First off, great repo. The code is well-written and I look forward to contributing.

So, from reading the source, I've gathered that this is how you would do this:

jekyll_s3_config = {}
jekyll_s3_config["s3_id"]     = s3_id
jekyll_s3_config["s3_secret"] = s3_secret
jekyll_s3_config["s3_bucket"] = s3_bucket
jekyll_s3_config["s3_endpoint"] = "us-east-1"

Jekyll::S3::Uploader.run(dir, jekyll_s3_config, false)

Notice the line where I set "s3_endpoint". If I use a .yml for config, "s3_endpoint", I don't need to include "es-east-1" as there is already a sane default tucked away here https://github.com/laurilehmijoki/jekyll-s3/blob/master/lib/jekyll-s3/config_loader.rb#L27.

So,

  1. I'm proposing altering this so you get this nice default without needing to use a .yml file.

  2. I'm proposing some README examples on accomplishing this.

Would you be interesting in this? If so, I'll try my hand at implementing and sending a PR.

laurilehmijoki commented

Hello adelevie

Regarding your point 1)

It is a good idea to move the inference of the default S3 endpoint from https://github.com/laurilehmijoki/jekyll-s3/blob/master/lib/jekyll-s3/config_loader.rb#L27 to somewhere near https://github.com/laurilehmijoki/jekyll-s3/blob/master/lib/jekyll-s3/uploader.rb#L9.

After this, you no longer need to define the endpoint in your own code.

Regarding your point 2)

Sounds good. The README.md is already quite lengthy, but it can hold a short example of how to use jekyll-s3 as a library. We can re-organise the documentation later on, if the need arises.

I welcome your pull request. Please take a look at https://github.com/laurilehmijoki/jekyll-s3/blob/master/features/jekyll-s3-as-library.feature. Can you add an example of your use case into that Cucumber file?

adelevie commented

Hey, thanks for the reply!

Looks good, I'm gonna start some work on this now. I'm totally new to Cucumber, so some patience might be needed!

adelevie commented

Not sure why I'm not seeing an option to submit this as a PR adelevie@2d825d7

adelevie commented

Also, I'm trying to make sure I can run the test suite so my PRs are reliable. I'm having trouble though :/

https://gist.github.com/adelevie/5338386

laurilehmijoki commented

As of version 2.4.3, when using jekyll-s3 as a library the s3_endpoint is no longer mandatory in _jekyll_s3.yml.

adelevie commented

Just wanted to thank you for you patience and acceptance of my PR! This is a great library and I look forward to continued contributions and collaborations.

laurilehmijoki commented

No problem, and thanks for your contributions!