Celluloid::Logger breaks existing code

[ioquatix]

In classes which include Celluloid::IO, if you write Logger.new in that context, you won't get ::Logger but Celluloid::Logger which can cause problems for existing code:

NoMethodError: undefined method `new' for Celluloid::Logger:Module

Let me know if you'd like me to write a failing test case.

[tarcieri]

Is using ::Logger not an acceptable workaround here?

Perhaps Celluloid could call this class Logging to avoid a namespace clash.

I'd recommend opening this issue on the Celluloid repo and not Celluloid::IO

[ioquatix]

Thanks for the quick reply.

I guess my main issue is that it is unexpected and if you are migrating existing code it (e.g. in this case RubyDNS) it's just another thing which needs to be changed/fixed.

From a technical perspective, using Celluloid changes the expected behaviour of a standard class name in a way which isn't compatible. If you need to expose it, I think using a different name might be a good idea, but if it is just an implementation detail, perhaps even make a module specific for that purpose or hide it in a different location altogether?

[tarcieri]

Certain things in the Celluloid(::IO) namespace we want to override core Ruby, or have short names, e.g. 'TCPSocket, orActor`

If names conflict with Ruby core classes and it isn't intentional, they should probably be changed

[ioquatix]

Yeah, that's what I was thinking so that's why I filed a bug report.

I like the magic part of this approach, but it is also borderline dysfunctional if you expect one thing and get another which isn't compatible, e.g. the recent UDPSocket#connect patch.

[digitalextremist]

Definitely being handled at celluloid/celluloid#420