Persist glob when replacing a path #6910

Merged
merged 1 commit into from Jul 1, 2012

Conversation

Projects
None yet
4 participants
Contributor

mulder commented Jun 29, 2012

When Rails::Paths::Root's []= is used to replace a path it should persist the previous path's glob. Without passing the glob along we get gnarly bugs when trying to wire up things like engines.

/cc @josevalim


From rails/engine.rb docs:

#   class MyEngine < Rails::Engine
#     paths["app/controllers"] = "lib/controllers"
#   end

This works great. but if you did the same with something that was configured with a glob, like app/initializers, you get errors like this:

lib/active_support/dependencies.rb:245:in `load': cannot load such file -- /Users/nick/Desktop/foo_engine/lib/foo_engine/initializers (LoadError)
module FooEngine
  class Engine < ::Rails::Engine
    isolate_namespace FooEngine

    config.paths['config/initializers'] = "lib/foo_engine/initializers"
  end
end

# Before the initializer override:
>> FooEngine::Engine.config.paths["config/initializers"].glob
=> "**/*.rb"

# After the initializer override:
>> FooEngine::Engine.config.paths["config/initializers"].glob
=> nil
Member

drogus commented Jun 29, 2012

Could you add a few lines of explanation on why this is needed in commit message? Here are guidelines on what should be in commit message: http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#commit-your-changes

Persist glob when replacing a path
When Rails::Paths::Root's []= is used to replace a path it should persist the previous path's glob. Without passing the glob along we get gnarly bugs when trying to wire up things like engines.

    module FooEngine
      class Engine < ::Rails::Engine
        isolate_namespace FooEngine

        config.paths['config/initializers'] = "lib/foo_engine/initializers"
      end
    end

    ## Example of behaviour before this commit.
    #
    # Before the initializer override:
    >> FooEngine::Engine.config.paths["config/initializers"].glob
    => "**/*.rb"

    # After the initializer override:
    >> FooEngine::Engine.config.paths["config/initializers"].glob
    => nil

    ## Example of behaviour after this commit.
    #
    # Before the initializer override:
    >> FooEngine::Engine.config.paths["config/initializers"].glob
    => "**/*.rb"

    # After the initializer override:
    >> FooEngine::Engine.config.paths["config/initializers"].glob
    => "**/*.rb"
Contributor

mulder commented Jun 30, 2012

@drogus done. Sorry, should have expanded on it before I pushed it up.

@mulder awesome commit message, thanks! 💚

Member

drogus commented Jul 1, 2012

@mulder no problem, we started asking for nicer commit messages not long ago and everyone here is guilty of bad commit messages ;)

josevalim added a commit that referenced this pull request Jul 1, 2012

Merge pull request #6910 from mulder/fix_path_glob
Persist glob when replacing a path

@josevalim josevalim merged commit 2ee3fa1 into rails:master Jul 1, 2012

carlosantoniodasilva added a commit that referenced this pull request Jul 1, 2012

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