Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What? Why?
There are three proposed changes.
The main motivation for this patch is that thanks to fxn/zeitwerk#282 and fxn/zeitwerk#284 I have learned
__dir__
is not a good path prefix for setting loaders up, because it is a real path, while__FILE__
is not. This detail shows up when using symlinks, as it is the case in the project those tickets were about. The proposed code usesFile.dirname(__FILE__)
instead, for cosistency in the involved paths.Gruf probably copied a snippet from the README that unrolled what
for_gem
does and used__dir__
, that snippet has been recently updated (fxn/zeitwerk@7daca61). However, that snippet is only there for illustration purposes, it is more simple and idiomatic to setup the loader withfor_gem
. So we simplify things a bit withfor_gem
.Finally, since I was on it,
Zeitwerk
is a top-level constant, there is no reason to use a leading::
in the top-level of the script.How was it tested?
Using demo apps locally, both auto and eager loading.