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
Add --fatal-deprecation
flag
#1346
base: main
Are you sure you want to change the base?
Conversation
This restructures deprecation warnings into a class that gives each deprecation an identifier and structures the warnings themselves primarily based on the instance variables of that Deprecation.
The core question is which deprecations are people likely to want to enable or disable in conjunction with one another. The main use-case for separating them out into separate identifiers in the first place is ensuring that users aren't broken by new deprecations, so coalescing multiple older deprecations isn't much of a problem. Given that, I'd make both the color unit deprecations use one identifier.
I think trying to factor out deprecation warnings into their constituent parts, while clever, might be a bit more complex than it needs to be. It would probably end up more flexible if you just use the
I'd prefer to avoid the zone-based approach as much as possible—I think it's generally a lot clearer to explicitly pipe things through the APIs. Here's an idea for an alternate way of handling this: define an class InternalLogger extends Logger {
/// Returns [logger] if it's an [InternalLogger], and wraps it in one otherwise.
static Logger wrap(Logger logger);
void warnForDeprecation(Deprecation deprecation, String message,
{FileSpan? span, Trace? trace});
} By default, this just forwards to That still leaves open the question of how to handle
I think if you make |
9cff4ca
to
6c59213
Compare
@jathak I think we can close this as it's already delivered in a different PR? |
Partial fix for #633.
This restructures deprecation warnings into a class that gives each deprecation an identifier and structures the warnings themselves primarily based on the instance variables of that Deprecation.
This supports the core use case of making a deprecation fatal from the command line, but I'd like some feedback on the general approach before finishing this PR (mainly adding support to the Dart API and updating any sass-specs with warnings that were tweaked as part of this).
My main questions are:
Deprecation
and the parameters forDeprecation.message
were mostly just based on what was necessary to replicate the existing warnings in a structured way. Are there any that don't make sense, or additional properties that should be added? Also, are there any deprecations without aremovedIn
version that should have one?withWarnCallback
to include anerror
function to allowDeprecation.warnOrError
to work? I'm more confident in the former, but I'm less sure about the latter.--fatal-deprecation
flag), or shouldDeprecation
be exposed as part of the public API?