Skip to content
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

write and verify rules.json #4590

Merged
merged 2 commits into from
Jul 21, 2023
Merged

write and verify rules.json #4590

merged 2 commits into from
Jul 21, 2023

Conversation

pq
Copy link
Member

@pq pq commented Jul 20, 2023

Fixes #4587

@parlough: let me know if this is what you had in mind?

/cc @bwilkerson @srawlins

@coveralls
Copy link

Coverage Status

coverage: 96.773%. remained the same when pulling 6c7bc93 on rules_json into 06c9c95 on main.

@parlough
Copy link
Member

parlough commented Jul 20, 2023

I'm happy with this! However it may not be what you want, since it will break if one of the lints packages modifies their list of lints or the error fix status map changes. Perhaps we want to adjust those to the latest stable tag/commit rather than main? Another benefit of that is it more closely matches what most users will experience.

Another option is adding a specific version of the lints and flutter_lints packages as dev dependencies and pulling the files from there.

@pq
Copy link
Member Author

pq commented Jul 20, 2023

Great points!

My gut is that in practice we won't get bitten here too often. My thinking being:

  1. lints and flutter_lints updates happen very infrequently, and while
  2. error_fix_status.yaml might change more frequently, it will become less of a hassle when we're co-located in the same repo and can make atomic changes

I guess I'm inclined to take the "simple" approach first and rethink if it becomes a problem down the road.

How does that sound?

@parlough
Copy link
Member

parlough commented Jul 20, 2023

Sounds good to me :)

Thanks for working on this!

@pq
Copy link
Member Author

pq commented Jul 20, 2023

Another benefit of that is it more closely matches what most users will experience.

Regarding this, won't the version of rules.json users generally interact w/ still be on dart.dev somewhere? And correspond to the latest stable release?

@pq pq requested review from srawlins and bwilkerson July 20, 2023 19:46
@parlough
Copy link
Member

parlough commented Jul 20, 2023

Regarding this, won't the version of rules.json users generally interact w/ still be on dart.dev somewhere? And correspond to the latest stable release?

It's kind of in an awkward spot for now. Since the analyzer still points to the the linter rule documentation rather than the diagnostic version for lints, I try to keep it up to date for main. That way new lints don't have a dead link for users on beta and dev. So for now I usually just manually update the fix status to match stable.

Once we switch to pointing to consistent diagnostic information, I don't think this will be a problem anymore.

@pq pq merged commit 61146fb into main Jul 21, 2023
6 checks passed
@pq pq deleted the rules_json branch July 21, 2023 17:54
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 23, 2023
* write and verify `rules.json`

* ++
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.

Preserve machine-generated linter rules JSON file
4 participants