-
-
Notifications
You must be signed in to change notification settings - Fork 103
Conversation
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A++ would read again
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 }` |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Less code to repeat ;)
There was a problem hiding this comment.
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
Any other concerns, @JonRowe? |
Nope, LGTM |
Add support for optimized requires.
See rspec/rspec-expectations#476
for the discussion and background to this.