Skip to content

Conversation

jvlara
Copy link

@jvlara jvlara commented Aug 19, 2025

Using warnings_treated_as_deprecation caused warn messages to be suppressed even when explicitly allowed.
This happened because the deprecator neither logs those messages nor saves them in the deprecation file.

How to test

  1. Remove the line added to the lib/deprecation_toolkit/warning.rb#deprecation_triggered? method.
  2. Run the tests and confirm that the new test fails.

@jvlara jvlara force-pushed the fix/warns-suppressed-even-when-allowed branch from e268430 to daeba3a Compare August 19, 2025 22:22
@jvlara jvlara changed the title fix: warns suppressed even when allowed fix: do not suppress allowed warn messages Aug 19, 2025
@jvlara jvlara force-pushed the fix/warns-suppressed-even-when-allowed branch from daeba3a to 20b4f8a Compare August 19, 2025 22:25
Using `warnings_treated_as_deprecation` caused `warn` messages
to be suppressed even if the message was explicitly allowed.
@jvlara jvlara force-pushed the fix/warns-suppressed-even-when-allowed branch from 20b4f8a to 324bc60 Compare August 19, 2025 22:29
@jvlara
Copy link
Author

jvlara commented Aug 19, 2025

I have signed the CLA!

@etiennebarrie
Copy link
Member

I think it's working as designed: when we're dealing with deprecations, either we collect them, or we don't want to see them.

Put another way, when a deprecation warning appears after a dependency was upgraded, it fails CI. We typically collect the deprecations and ship that, and later start working on them on subsequent PRs (removing the deprecated code and the collected deprecations as we go).
Sometimes there are too many tests impacted, or for some other reason we think we don't need to collect them, then we don't want to see the warnings either.

So at the very least this needs to be a configuration option and not just the new behavior.

@jvlara
Copy link
Author

jvlara commented Aug 20, 2025

I think I understand your point.

What I’m running into is that some gems (for example, tapioca
) use warn for errors or diagnostics that are not actually "deprecations". When warnings_treated_as_deprecation is enabled for every warn with "//", those messages are currently treated as deprecations as well, which feels a bit too broad.

One idea would be to add a configuration option, something like ignore_warnings_treated_as_deprecations, which could list patterns (or procs) for warn messages that should not be considered deprecations. That way the default “treat all warns as deprecations” behavior stays intact, but projects that need to exempt specific cases (like gems using warn differently) can opt out for those messages.

@jvlara
Copy link
Author

jvlara commented Aug 20, 2025

With this setup i was able to disable that behaviour using a complex regex

IGNORED_WARNINGS_AS_DEPRECATIONS = ["rbi"]
exceptions = IGNORED_WARNINGS_AS_DEPRECATIONS.join("|")
regex_for_warnings_as_deprecations = /\A(?!.*(#{exceptions})).*\z/m
DeprecationToolkit::Configuration.warnings_treated_as_deprecation = [regex_for_warnings_as_deprecations]
# instead of 
# DeprecationToolkit::Configuration.warnings_treated_as_deprecation = [//]

If you don't think this add any value, just close the PR 👌🏼

@jvlara jvlara closed this Aug 22, 2025
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