-
Notifications
You must be signed in to change notification settings - Fork 725
Include linter rules on Dart.dev #3064
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
Conversation
579a23b
to
2d5f668
Compare
This is ready for review. At first, I don't plan to replace the external(analyzer) linkages to this page. The goal is to get this in a good state to merge, then I can incrementally work on the necessary improvements in smaller follow-up pull requests. This gives time to identify issues and will make reviewing easier. |
This looks great. fyi @mit-mit as this bears on team-recommended lints. @parlough: is the json a copy of what's in the linter gh-pages branch? https://github.com/dart-lang/linter/blob/gh-pages/lints/machine/rules.json What are your thoughts about keep that all synced up? |
@pq Ah that's where the generated file is. I regenerated it manually since I forgot where I found it originally. I will keep it up synced up from there for now, but I'm wondering if we can create a GitHub action(on here or linter repo) with a cron job that checks if the file is updated and makes a commit or PR on this repository. |
Yeah, some auto-sync would be good. Not sure about automating commits -- never looked into it! Alternatively, you could add a check that just fails the build when they've gone out of sync? (Not as awesome but at least lets you know when things have gone stale...) |
748a79f
to
d7d4000
Compare
Rebased and staged at https://parlough-dart-dev.web.app/tools/linter-rules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a bunch of nitpicky word comments, but I love the way you've organized the page!
I don't understand why my review comments appeared in the order they did... but hopefully you can make sense of them. |
Co-authored-by: Kathy Walrath <[email protected]>
e2709e9
to
f9bcba0
Compare
@kwalrath Thanks for the thorough review and comments, I appreciate them a lot! I went through and included your changes as well as made some other minor adjustments. Please take another look when you get a chance :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I have a question for @mit-mit that should be answered before we ship this page.
For example, some rules are more appropriate for library packages, | ||
and others are designed for Flutter apps. | ||
**Consider starting with pre-packaged linter rules**, | ||
such as those in the [`effective_dart` package][]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mit-mit should we point to https://pub.dev/packages/lints instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was planning on a follow-up change for once lints has a 1.0.0 version released documenting the different types as well to make all those changes easier to review, but I'd be happy to include them in this PR instead. Just let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I say let's publish this now and file an issue reminding us to update the package!
Co-authored-by: Kathy Walrath <[email protected]>
The goal is to standardize hosting rule information on dart.dev similar to the analyzer diagnostics. As a result, the style followed implementing this followed closely to that.
Parker's must complete tasks:
Long term tasks for after landing:
You can find a staged version of this here:
Last updated 05-09-21
https://parlough-dart-dev.web.app/tools/linter-rules