samuelkadolph / unicorn-rails

unicorn-rails is a simple gem that sets the default server for rack (and rails) to unicorn.
MIT License
127 stars 22 forks source link

remove redundant declarations around require() #4

Closed sunaku closed 11 years ago

samuelkadolph commented 11 years ago

They aren't redundant, they serve a purpose of making sure the version.rb namespace never changes and can't cause problems down the road without tests catching it. It's less likely to happen in a small project like this but in a larger one it can happen and it's hard to track down unless you've done it before.

sunaku commented 11 years ago

Putting a require statement inside a namespace does not cause the required code to be scoped therein:

$ irb
## ruby 1.9.3p392 (2013-02-22 revision 39386) [x86_64-linux]
>> module Foo
>>   module Bar
>>     require "unicorn/rails/version"
>>   end
>> end
true
>> Unicorn::Rails::VERSION
"1.0.1"
>> Foo::Bar::Unicorn::Rails::VERSION
NameError: uninitialized constant Foo::Bar::Unicorn
        from (irb):7
        from /home/skurapati/.rvm/rubies/ruby-1.9.3-p392/bin/irb:13:in `<main>'

Notice that the Foo::Bar namespace had no effect on the require statement. If it did, then there would have been a Foo::Bar::Unicorn::Rails::VERSION constant afterward and not a Unicorn::Rails::VERSION constant.

Therefore, the lines of code removed in this pull request are redundant and can be safely removed. :neckbeard:

samuelkadolph commented 11 years ago

That's not what I meant. The problem can come up when you change the namespace of your gem but not update version.rb which can cause problems like superclass mismatch or X is not a class/module.

sunaku commented 11 years ago

In that situation, your version.rb will continue to define the Unicorn::Rails namespace (as it currently does) and the Unicorn::Rails::VERSION constant too. The problems you mentioned will not be solved by enclosing the require statement inside your newly renamed namespace (or inside any namespace for that matter).

samuelkadolph commented 11 years ago

This is the style I used for my gems now because I've been bitten in the pass when changing the root namespace for the gem and the version craps all over that. I don't see any advantages to removing the module.