codereading / rack

a modular Ruby webserver interface
http://rack.rubyforge.org/
Other
18 stars 2 forks source link

Not follow standard filename convention #5

Open samnang opened 12 years ago

samnang commented 12 years ago

Following Ruby standard filename convention, filename should be all in lowercase and separate each word by underscore, but I see some filenames break this rule:

commonlogger.rb      # CommonLogger
methodoverride.rb    # MethodOverride
showexceptions.rb    # ShowExceptions

I feel it should be

common_logger.rb     # CommonLogger
method_override.rb   # MethodOverride
show_exceptions.rb   # ShowExceptions

Do have any ideas why they have filenames like this?

skade commented 12 years ago

Historical reasons, I guess. Rack is a pretty old library and this naming convention wasn't as strict back then (I am not even sure whether it wasn't just considered a "Rails convention"). Changing the file name of commonlogger.rb would break a lot of stuff.

tomykaira commented 12 years ago

I noticed that Rack takes capitalized hash key like :Host, :Port, and :AccessLog(https://github.com/codereading/rack/blob/rack-1.4/lib/rack/server.rb#L43).

Are these historical, too? They are queer to me.

skade commented 12 years ago

I think the case here is a bit different:

https://github.com/codereading/rack/blob/rack-1.4/lib/rack/server.rb#L142-173

As you can see, most options are downcased, except the options that are directly passed to the Handler. I think its a kind of "namespacing".

tomykaira commented 12 years ago

I see. Thanks @skade !

samnang commented 12 years ago

Changing the file name of commonlogger.rb would break a lot of stuff.

Why that could be happen? I see it load by using autoload :CommonLogger, "rack/commonlogger" in lib/rack.rb, so other libraries could require 'rack' and use this middleware by calling its classname use Rack::CommonLogger without any problems. I don't see the point why other libraries require 'rack/commonlogger' directly like that.

skade commented 12 years ago

One reason might be that autoload is inherently thread-unsave and should not be used. It is also deprecated and flagged for removal[1]. Also, even if autoload hooks are in place, libs are still free to require commonlogger.rb by hand, which some do to ensure load order. I do so regularly to avoid autoload issues which blow up in my face in regular intervals and would recommend to do so in any case.

[1] http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/41149