Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Use require_relative inplace of standard require if available. #502

Merged
merged 1 commit into from
Nov 13, 2011

Conversation

ileitch
Copy link
Contributor

@ileitch ileitch commented Nov 13, 2011

As per discussion on the mailing list: http://rubyforge.org/pipermail/rspec-users/2011-November/020760.html

Possibly the simplest approach I could think of. Provided this is OK, it could probably be moved into its own file.

dchelimsky added a commit that referenced this pull request Nov 13, 2011
Use require_relative inplace of standard require if available.
@dchelimsky dchelimsky merged commit 002f6b7 into rspec:master Nov 13, 2011
dchelimsky added a commit that referenced this pull request Nov 13, 2011
Using require_relative instead of require results in a roughly 12%
improvement in the time it takes to load rspec-core's own lib files
dchelimsky added a commit that referenced this pull request Nov 13, 2011
@dchelimsky
Copy link
Contributor

@ileitch - this seems good to me with defined? rather than Kernel.respond_to? (feels cleaner, and turns out to shave off a millionth of a second to boot!).

Once concern I have is that we're doing this in the global namespace. What do you think about something like:

module RSpec
  if defined?(require_relative)
    def _require(path)
      require_relative path
    end
  else
    def _require(path)
      require "rspec/#{path}"
    end
  end
  module_function :_require
end

RSpec._require 'core/filter_manager'
# etc

I'd want the _ prefix or similar to identify it for readers as internal to rspec, and avoid collisions with Kernel methods.

As we add this to the other libs, we'd do something more like:

module RSpec
  unless defined?(_require)
    if defined?(require_relative)
      def _require(path)
        require_relative path
      end
    else
      def _require(path)
        require "rspec/#{path}"
      end
    end
  end
  module_function :_require
end

That way we wouldn't be redefining it as each lib got loaded.

Thoughts?

@ileitch
Copy link
Contributor Author

ileitch commented Nov 13, 2011

namespacing and defined? both look sound to me.

@dchelimsky
Copy link
Contributor

Let's do it. So we need to make this change in rspec-core and then add the whole thing to the other rspec libs. Would you like to do any of that?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants