Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Moved default location constraint logic from config loader to endpoint #34

Closed
wants to merge 6 commits into from

2 participants

@adelevie
Collaborator

This will allow Jekyll::S3::Uploader to be run programmatically while providing a sane default that you'd get from a yml config file.

I cannot get the full test suite to run properly, but running the affected specs, tests pass:

adelevie@Alans-MacBook-Air:jekyll-s3 → master rspec spec/lib/endpoint_spec.rb 
.

Finished in 0.00105 seconds
1 example, 0 failures
adelevie@Alans-MacBook-Air:jekyll-s3 → master rspec spec/lib/config_loader_spec.rb 
..

Finished in 0.00567 seconds
2 examples, 0 failures

Notice that I completely removed a test from the config loader spec and moved its equivalent to a new file, endpoint_spec.rb.

@adelevie
Collaborator

Here's the error travis is displaying:

/home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/minitest/unit.rb:581:in `block in process_args': invalid option: --tags (OptionParser::InvalidOption)
    from /home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/minitest/unit.rb:560:in `new'
    from /home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/minitest/unit.rb:560:in `process_args'
    from /home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/minitest/unit.rb:591:in `run'
    from /home/travis/.rvm/rubies/ruby-1.9.2-p320/lib/ruby/1.9.1/minitest/unit.rb:508:in `block in autorun'
rake aborted!
Command failed with status (1): [bundle exec cucumber --tags ~@skip-on-trav...]

It's only for Ruby 1.9.2 (all other versions pass), and I have no idea how this relates to the code I submitted.

@laurilehmijoki

Hi Alan

I merged your commit 3eec3f1. I did not merge the other commits, since they seemed like work-in-progress commits. Generally, it's a good idea to include in the pull request only the commits you wish to see in the next release.

Regarding Cucumber and VCR: they are tools that one needs to spend a bit time with if one wants to know how to use them. I did not add new Cucumber examples. Instead, I added a couple of more RSpec examples: 801499c and bcd7fa1.

I also added readme on how to use jekyll-s3 as a library: https://github.com/laurilehmijoki/jekyll-s3#using-jekyll-s3-as-a-library. Is the readme fine to you?

Anyway, thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  features/jekyll-s3-as-library.feature
@@ -20,4 +20,4 @@ Feature: Using Jekyll-s3 as a library
Scenario: Developer wants feedback on how many Cloudfront items Jekyll-s3 invalidated
When my Jekyll site is in "features/support/test_site_dirs/cdn-powered.with-one-change.blog.fi"
Then jekyll-s3 will push my blog to S3 and invalidate the Cloudfront distribution
- And report that it invalidated 2 Cloudfront item
+ And report that it invalidated 2 Cloudfront item
View
1  lib/jekyll-s3/config_loader.rb
@@ -24,7 +24,6 @@ def self.check_s3_configuration(site_dir)
# Raise MalformedConfigurationFileError if the configuration file does not contain the keys we expect
def self.load_configuration(site_dir)
config = load_yaml_file_and_validate site_dir
- config['s3_endpoint'] = config['s3_endpoint'] || 'us-east-1'
return config
end
View
6 lib/jekyll-s3/endpoint.rb
@@ -1,9 +1,11 @@
module Jekyll
module S3
class Endpoint
+ DEFAULT_LOCATION_CONSTRAINT = 'us-east-1'
attr_reader :region, :location_constraint, :hostname, :website_hostname
- def initialize(location_constraint)
+ def initialize(location_constraint=nil)
+ location_constraint = DEFAULT_LOCATION_CONSTRAINT if location_constraint.nil?
raise "Invalid S3 location constraint #{location_constraint}" unless
location_constraints.has_key?location_constraint
@region = location_constraints.fetch(location_constraint)[:region]
@@ -25,6 +27,6 @@ def location_constraints
'sa-east-1' => { :region => 'South America (Sao Paulo)', :website_hostname => 's3-website-sa-east-1.amazonaws.com', :endpoint => 's3-sa-east-1.amazonaws.com' }
}
end
- end
+ end
end
end
View
5 spec/lib/config_loader_spec.rb
@@ -8,11 +8,6 @@
config['s3_bucket'].should eq('galaxy')
end
- it 'uses the "us-east-1" as the default endpoint' do
- config = Jekyll::S3::ConfigLoader.load_configuration('spec/sample_files/hyde_site/_site')
- config['s3_endpoint'].should eq('us-east-1')
- end
-
it 'reads the S3 endpoint setting from _jekyll_s3.yml' do
config = Jekyll::S3::ConfigLoader.load_configuration('spec/sample_files/tokyo_site/_site')
config['s3_endpoint'].should eq('ap-northeast-1')
View
11 spec/lib/endpoint_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+require 'pp'
+
+describe Jekyll::S3::Endpoint do
+
+ it 'uses the "us-east-1" as the default location' do
+ endpoint = Jekyll::S3::Endpoint.new
+ endpoint.location_constraint.should eq(Jekyll::S3::Endpoint::DEFAULT_LOCATION_CONSTRAINT)
+ end
+
+end
Something went wrong with that request. Please try again.