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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] make Ruby#setWarningsEnabled actually useful #7728

Merged
merged 13 commits into from
Apr 5, 2023

Conversation

kares
Copy link
Member

@kares kares commented Mar 16, 2023

runtime.setWarningsEnabled() in it's current form is actually useless given typical Ruby apps usage of Kernel.silence_warnings and $VERBOSE = .... so I tried to re-invent the idea to actually make it useful:

setting runtime.setWarningsEnabled(false) would act as a kill switch (typically in a production env) to avoid nasty surprises esp. with embedded JRuby scenarios - regardless of gems fiddling with $VERBOSE warnings end up disabled.

second I reviewed warning calls to use warning method, have an ID and report proper line number (some were of due the +1/-1 going on) by moving the line adjustment logic to the "source". also, RubyWarnings got refactored so that the warning/warn calls delegate to pretty much one method as I was thinking of adding a getWarningFilter on the runtime (it's just a few lines to add and works good but I am not yet sure whether it would be worth maintaining given the introduction of the warning gem 馃).

@kares kares self-assigned this Mar 16, 2023
@kares kares marked this pull request as ready for review March 23, 2023 14:18
@kares kares linked an issue Apr 4, 2023 that may be closed by this pull request
@kares kares added this to the JRuby 9.3.11.0 milestone Apr 4, 2023
@kares kares modified the milestones: JRuby 9.3.11.0, JRuby 9.4.3.0 Apr 4, 2023
@kares kares merged commit 98c9d13 into jruby:master Apr 5, 2023
47 of 48 checks passed
@kares kares deleted the user-warnings branch April 5, 2023 10:20
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.

some line numbers on warnings are off by 1
2 participants