8000 Avoid some false positive bundled gem warnings by deivid-rodriguez · Pull Request #12786 · ruby/ruby · GitHub
[go: up one dir, main page]

Skip to content

Avoid some false positive bundled gem warnings #12786

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deivid-rodriguez
Copy link
Contributor

When a bundled gem has already been removed, a require caller is rescuing LoadError, no warning/error messages should be displayed.

Instead, let the bundled gem message be part of LoadError, so that it's not displayed when rescued, but still gets to the user when not rescued.

This is an idea to reintroduce #11550, but without the issue that made @hsbt revert the original approach: If users upgrade Ruby from a version that did not show any warnings, to a version where the default gem has already been removed, then they'll miss any noticed about removal of the default gem. By making the notice part of the LoadError, they will not miss it.

8000

Copy link
launchable-app bot commented Feb 20, 2025

Tests Failed

✖️no tests failed ✔️61956 tests passed(37 flakes)

@deivid-rodriguez deivid-rodriguez force-pushed the optional-gems-warnings branch from c819aef to 5a7392d Compare April 8, 2025 14:19
@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review April 8, 2025 14:19
When a bundled gem has already been removed, a `require` caller is
rescuing `LoadError`, no warning/error messages should be displayed.

Instead, let the bundled gem message be part of `LoadError`, so that
it's not displayed when rescued, but still gets to the user when not
rescued.
@deivid-rodriguez deivid-rodriguez force-pushed the optional-gems-warnings branch from 5a7392d to 37a4551 Compare June 12, 2025 14:31
@deivid-rodriguez
Copy link
Contributor Author

This still feels like a small usability improvement to me, even if it does not fix all issues. Thoughts?

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.

1 participant
0