Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CDN support #451

Merged
merged 1 commit into from Sep 6, 2014

Conversation

Projects
None yet
3 participants
@kost
Copy link
Contributor

commented Jun 17, 2014

Implemented support for CDN

if cdn_id is specified, it will invalidate objects on CDN.

Implemented fixes as Fog does not support dry run. Also, basic test is implemented.

@coveralls

This comment has been minimized.

Copy link

commented Jun 17, 2014

Coverage Status

Coverage decreased (-0.02%) when pulling 082372d on kost:cdn-support into 1ab8a19 on nanoc:master.

@kost

This comment has been minimized.

Copy link
Contributor Author

commented Jul 28, 2014

Any problem with the pull/patch?

@ddfreyne

This comment has been minimized.

Copy link
Member

commented Jul 29, 2014

My apologies. I haven’t had the time to review it yet.

@@ -65,6 +72,7 @@ def run
files = files + set
end
keys_to_destroy = files.all.map { |file| file.key }
keys_to_invalidate = Array.new

This comment has been minimized.

Copy link
@ddfreyne

ddfreyne Sep 6, 2014

Member

Idiomatic Ruby uses [] instead of Array.new.

@@ -86,6 +95,21 @@ def run
directory.files.get(key).destroy
end

# invalidate CDN objects
if cdn_id then

This comment has been minimized.

Copy link
@ddfreyne

ddfreyne Sep 6, 2014

Member

Idiomatic Ruby doesn’t use the then after the if. (There are some similar instances below.)

keys_to_invalidate.concat keys_to_destroy
cdn = ::Fog::CDN.new(config)
# fog cannot mock CDN requests
if not self.dry_run? then

This comment has been minimized.

Copy link
@ddfreyne

ddfreyne Sep 6, 2014

Member
if not self.dry_run? then

should become

unless dry_run?
# invalidate CDN objects
if cdn_id then
puts 'Invalidating CDN distribution'
keys_to_invalidate.concat keys_to_destroy

This comment has been minimized.

Copy link
@ddfreyne

ddfreyne Sep 6, 2014

Member

Method calls that accept parameters should use parentheses.

(There are some exceptions to this, but puts and require are the only ones I can think of at the moment).

@kost

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2014

Thanks. implemented all suggestions. Let me know if there's anything else.

@ddfreyne

This comment has been minimized.

Copy link
Member

commented Sep 6, 2014

Looks good otherwise. Can you squash your six commits together? I’ll merge afterwards.

@ddfreyne ddfreyne added waiting and removed to review labels Sep 6, 2014

@kost kost force-pushed the kost:cdn-support branch from 54a41ad to efa5cab Sep 6, 2014

@kost

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2014

That should be it.

@ddfreyne

This comment has been minimized.

Copy link
Member

commented Sep 6, 2014

Thanks!

ddfreyne added a commit that referenced this pull request Sep 6, 2014

@ddfreyne ddfreyne merged commit 7f2b9b1 into nanoc:master Sep 6, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@ddfreyne ddfreyne removed the waiting label Sep 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.