search

fxn / zeitwerk

Efficient and thread-safe code loader for Ruby
MIT License
1.97k stars 117 forks source link

Remove unused call to `register_explicit_namespace` #256

Closed shioyama closed 1 year ago

shioyama commented 1 year ago

AFAICT the autoload_path here is always a directory, so ruby?(autoload_path) is always false, hence we can remove this. Tests pass with the removal.

fxn commented 1 year ago

Not really!

That logic is triggered when you have this situation:

app/controllers/foo.rb
app/models/foo/x.rb

If app/controllers is first in the autoload paths, scanning finds app/controllers/foo.rb first, and with the information it has at the moment, that is a regular file for which you have to set an autoload.

Later on, it finds the directory app/models/foo. This additional data tells the loader hotel.rb was actually defining an explicit namespace, and adjusts the internal metadata accordingly.

However, your patch made me realize test coverage for this was missing, and also a code comment. I have done this in https://github.com/fxn/zeitwerk/commit/3704206155166a119d5c134484cefd4303bc4097.

BTW, I see the test suite prints a few warnings if running with Ruby 3.2.0, I'll investigate them, the suite should run clean and silent.

shioyama commented 1 year ago

Makes sense, thanks! Glad I at least increased test coverage :slightly_smiling_face: