-
Notifications
You must be signed in to change notification settings - Fork 23
proposal: remove lints deprecated for 3.0 #789
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
Comments
|
Regarding mechanics, how about a
(EDIT: talking to @srawlins, we may want a release before that includes (EDIT 2: see below for a new proposal that does not bump the lower bound.) |
I don't think there is anything breaking about removing lints from lists. I think that could be Perhaps that is not safe because they could have opted-out files, running Dart 2.17, and those opted out files would no longer receive the benefit of those lint rules. |
EDIT: new proposal. Having thought about this some more and talking it over with @bwilkerson at length, I have a new simpler proposal: A Risk. There is a risk that folks who have opted libraries to pre-2.12 will lose some coverage. (I expect this to be extremely uncommon in practice.) How do folks feel? /cc @mit-mit @munificent @lrhn @natebosch @jakemac53 @goderbauer @jacob314 @bwilkerson @srawlins |
Would this be after we finish the internal migration? E.g. |
Is it enabled by including one of these lint sets? If so, then we'd need to enable it independently of these lint sets before removing it from the lint set. If not, then there's no reason to wait. |
Haha good question Brian. Total brain fart. We don't use these lint sets so just ignore me. 😄 No problem for internal use. |
Have we considered just making the lints themselves be no-ops based on language version? It seems to me that they will still have value for a long time given packages may be on an old language version for quite some time (O(years)?). |
Yes. But we want to let people know that they can remove them from their analysis options files just to help keep the world a little cleaner. But we can't let them know if they're still in the core, recommended, or flutter sets because then there would be too much noise. Hence the desire to remove them from these sets. |
@jakemac53 that's essentially what we've been doing but to Brian's point there's some value in tidying up. Also, to be super clear, these lints are only valuable in libraries that opt back pre |
The suggestion in #789 SGTM. Presumably, we'd also do a 2.1.0 release of flutter_lints that just bumps the package:lints version to 2.1.0? Are there any additional lints in flutter_lints that have become obsolete and should be removed as well? The list of those lints is here: https://github.com/flutter/packages/blob/main/packages/flutter_lints/lib/flutter.yaml. I believe these are all still relevant even with Dart 3.0. Just double-checking... |
Actually, flutter_lints already specifies |
@goderbauer: I think all the flutter-specific lints are good. As for a version bump, I don't know if it's necessary but don't feel really strongly either way! |
I wouldn't expect to get a lint for an included set of lints - the lint should only show up where they are explicitly included (so the core lint sets themselves). We could add a comment there for the reasoning and call it good? |
I think that's a great idea, and after talking with Phil that's the approach we've decided to take. Thanks! |
💯 Thanks @jakemac53! |
See: https://github.com/dart-lang/lints/issues/95 Change-Id: Ibc71c71027dc6d7b057e1caf201694394461e01c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280123 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Phil Quitslund <[email protected]>
@pq are you working on this one? |
There's a question about dart-lang/sdk#58958 to @srawlins. Once we resolve that we can push ahead here I think. |
OK. Revisiting this, my new thinking is that we only want to remove CL coming. |
@pq is this done? |
At this point, we just need to land dart-archive/lints#107 and publish. Since this bumps the lower SDK bound to 3.0.0 I was imagining we'd wait until a time closer to when such an SDK exists but maybe that's silly? |
@pq with the last beta done, I think we can go ahead and publish? https://dart.dev/tools/pub/publishing#publishing-prereleases |
I think so. It sounds like I should land dart-archive/lints#107 and then publish it as a preview? |
Commented in dart-archive/linter#107 |
Thanks all. dart-archive/linter#107 is landed and we're newly published at https://pub.dev/packages/lints/versions/2.1.0. Closing as done. |
dart-lang/sdk#58947 tracks issues that are being discussed for deprecation in 3.0.
Currently
43 of them are core/recommended.always_require_non_null_named_parameters
[recommended]deprecate:null_closures
[recommended]prefer_equal_for_default_values
[recommended] (as of 2.19 analyzer produces a diagnostic; discussion in Version bump. dart-archive/linter#92)Deprecate(DELAYED)iterable_contains_unrelated_type
andlist_remove_unrelated_type
sdk#58958 [core] (in favor ofcollection_methods_unrelated_type
-- Adderrors
to builtin groups. dart-archive/linter#93)There's an open question about
null_closures
so that may come off the list. (EDIT: yes.)Two things I'd like to establish:
package:lints
release. (Do we want to have version scoped releases? That is, a release with an SDK lower bound of3.0.0
or similar?)/cc @mit-mit @munificent @lrhn @natebosch @jakemac53 @goderbauer for feedback
/fyi @jacob314 @bwilkerson @srawlins
The text was updated successfully, but these errors were encountered: