Fixes public root bug with Jammit 0.6.5 #85

Merged
merged 1 commit into from Feb 2, 2012

Projects

None yet

2 participants

@vesan

The way Jammit defines the public root was changed in
documentcloud/jammit@b6ff4f0

This patch handles both the new and old implementation.

@vesan vesan Fixes public root bug with Jammit 0.6.5
The way Jammit defines the public root was changed in
documentcloud/jammit@b6ff4f0
ec63310
@gabebw gabebw commented on the diff Feb 1, 2012
spec/kumade/packagers/jammit_packager_spec.rb
@@ -7,7 +7,8 @@
it_should_behave_like "packager"
- its(:assets_path) { should == File.join(Jammit::PUBLIC_ROOT, Jammit.package_path) }
+ let(:jammit_public_root) { defined?(Jammit.public_root) ? Jammit.public_root : Jammit::PUBLIC_ROOT }
+ its(:assets_path) { should == File.join(jammit_public_root, Jammit.package_path) }
@gabebw
gabebw Feb 1, 2012

Could you rewrite this so that jammit_public_root isn't a global (within the describe block) variable? Something like this:

it "has the correct asset path" do
  jammit_public_root = defined?(Jammit.public_root) ? Jammit.public_root : Jammit::PUBLIC_ROOT
  subject.assets_path.should == File.join(jammit_public_root, Jammit.package_path)
end

That way it's clear that only one test is using the variable.

@gabebw
thoughtbot, inc. member

Thanks for fixing this! Just one comment, then you'll be good to merge.

@gabebw
thoughtbot, inc. member

Actually, I'm going to merge this anyway.

@gabebw gabebw merged commit a3bd6af into thoughtbot:master Feb 2, 2012
@gabebw gabebw referenced this pull request Feb 2, 2012
Closed

Get to green #86

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment