Repository navigation
Integrate package:flutter_lints into templates - #81417
Conversation
flutter-dashboard
Bot
added
the
tool
|
/cc @mit-mit @pq @devoncarew @Hixie for discussion |
There was a problem hiding this comment.
You could link to https://dart.dev/tools#ides-and-editors
There was a problem hiding this comment.
nit: consider removing the period in the url to avoid copy/paste issues
There was a problem hiding this comment.
if we're landing before this gets fixed, let's put the TODO (and maybe the commented-out code?) somewhere where it won't get stuck in people's analysis options for years.
There was a problem hiding this comment.
I will fix this before submitting it. The Fix has already landed in the Dart SDK and engine, just waiting for a roll into the framework.
There was a problem hiding this comment.
"automatically upgrade" maybe?
There was a problem hiding this comment.
isn't this redundant with the analysis options? i'm not sure i understand how they relate. if we really need both we should definitely explain the difference in these files.
There was a problem hiding this comment.
Reworded the paragraph above. This line just makes the set of recommended lints available. The import in the analysis_options.yaml then activates that set.
goderbauer
changed the title
There was a problem hiding this comment.
Does this conflict with the private return types lint? Or is that not in the recommended set
There was a problem hiding this comment.
That's not in the recommended set.
|
Thank you! Submission is now blocked on the tree becoming green and dart-lang/sdk#58383 rolling into the framework. |
I believe the fix for this landed in dart-lang/sdk (dart-lang/sdk@f193637) ~8 days ago, so should likely already be available in flutter/flutter. |
While the fix has rolled into the engine, it's not available in flutter/flutter yet due to an unfortunate combination of engine rollbacks and tree closures. I believe the next engine roll into the framework will contain the fix, though, once the tree is open again. My newly added tests is actually currently failing because the fix is not rolled in yet: https://github.com/flutter/flutter/pull/81417/checks?check_run_id=2520340935 |
|
This broke skp_generator tests. |
Could parts of that test run presubmit (e.g. at least analyzing the code) to avoid this in the future? |
|
skp_generator should already be running presubmit |
|
I don't see it there though, weird. |
|
It was added in #81096 |
|
Fix in #82239 |
| expect(exec.exitCode, 0); | ||
| expect(exec.exitCode, isNot(0)); | ||
| String lineParser(String line) { | ||
| print('#$line#'); |
There was a problem hiding this comment.
was this a temporary debugging line that leaked in?
There was a problem hiding this comment.
Oh, yeah. Will send PR to remove.
8 participants
Part of #78432.
This PR enables the recommended set of lints for Flutter apps and packages from
package:flutter_lintsfor apps/packages/plugins created viaflutter create. It also adapts those templates to ensure that they are analyzer clean under the new set of lints.There's a bug in the linter wrt treatment of empty rules sections that is fixed upstream, but not rolled into the framework yet: dart-lang/sdk#58383