Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use require_relative #90

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Use require_relative #90

merged 1 commit into from
Apr 23, 2024

Conversation

koic
Copy link
Contributor

@koic koic commented Apr 20, 2024

This PR uses require_relative instead of require. It prevents redundant searches due to the size of $LOAD_PATH.

NOTE: Changes that seem to eliminate the need for require are addressed in #89.

@mtsmfm
Copy link
Owner

mtsmfm commented Apr 22, 2024

@koic Thank you, can you rebase the branch onto the latest main?
Also you have a typo on both commit message and title 😉

@koic
Copy link
Contributor Author

koic commented Apr 22, 2024

Thank you for the review. It seems possible to update the require to require_relative as seen in PR #89, in addition to the current PR changes. I can include that update in this PR if it suits. Are there any concerns?

@koic koic changed the title Use rquire_relative Use require_relative Apr 22, 2024
This PR uses `require_relative` instead of `require`.
It prevents redundant searches due to the size of `$LOAD_PATH`.
@koic koic force-pushed the use_require_relative branch from e66debf to d0237c4 Compare April 22, 2024 15:50
@koic
Copy link
Contributor Author

koic commented Apr 22, 2024

I've updated the changes as mentioned above. Please let me know if there are any issues, and I will revert them.

@mtsmfm
Copy link
Owner

mtsmfm commented Apr 23, 2024

It seems the benchmark workflow has a problem with forked repo though, I'll merge this PR. Thanks!

@mtsmfm mtsmfm merged commit cd31588 into mtsmfm:main Apr 23, 2024
15 of 18 checks passed
@koic koic deleted the use_require_relative branch April 23, 2024 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants