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

Add support for optimized requires. #45

Merged
merged 3 commits into from
Feb 24, 2014
Merged

Conversation

myronmarston
Copy link
Member

See rspec/rspec-expectations#476
for the discussion and background to this.

See rspec/rspec-expectations#476
for the discussion and background to this.
# `require_relative` is preferred when available because it is always O(1),
# regardless of the number of dirs in $LOAD_PATH. `require`, on the other
# hand, does a linear O(N) search over the dirs in the $LOAD_PATH until
# it can resolve the file relative to one of the dirs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A++ would read again

@myronmarston
Copy link
Member Author

Any other concerns or is this good to merge, @xaviershay?

module RSpec
module Support
# @api private
#
# Defines a helper method that is optimized to require files from the
# named lib. The passed block MUST be `{ |f| require_relative f }`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear explanation, but that is a weird declaration requirement 😀

Now I know another good use case for the binding_of_caller gem… (but since we control all the code here, there's really no need).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about passing in __FILE__ and building a require to the absolute path, which should also shortcut the lookup?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wondered about passing in FILE and building a require to the absolute path, which should also shortcut the lookup?

I think that would probably work, but I dislike it for a few versions:

  • Years of doing ruby has taught me that absolute requires are rarely what you want.
  • Mixing absolute and relative requires can often lead to pain, where it allows a file to be loaded twice if you reference it in two different ways
  • require_relative is the 1.9+ way of doing this; why re-invent it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Less code to repeat ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but Ok I'm convinced

@myronmarston
Copy link
Member Author

Any other concerns, @JonRowe?

@JonRowe
Copy link
Member

JonRowe commented Feb 24, 2014

Nope, LGTM

myronmarston added a commit that referenced this pull request Feb 24, 2014
Add support for optimized requires.
@myronmarston myronmarston merged commit f36bbda into master Feb 24, 2014
@myronmarston myronmarston deleted the define-optimized-require branch February 24, 2014 22:08
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.

4 participants