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

Automatically check for new Dart lints #284

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

chykon
Copy link
Contributor

@chykon chykon commented Feb 24, 2023

Description & Motivation

Since the lints for Dart is updated quite often and no official package that provides a complete set of rules, the problem of automating tracking arises. This pull request contains changes to address this issue without using third-party dependencies.

It is proposed to add a step to check for new rules in CI and in the tool/run_checks.sh script.

ISSUES

After updating analysis_options.yaml, new rules-related issues have been identified:

The first rule has been temporarily disabled due to file header peculiarities (#264). For the other two, changes will be provided in a separate PR.

Related Issue(s)

#264

Testing

See step Check Dart lints:

Backwards-compatibility

Is this a breaking change that will not be backwards-compatible? If yes, how so?

Yes, because of the new rules, dart analyze throws an error.

Documentation

Does the change require any updates to documentation? If so, where? Are they included?

No.

@mkorbel1
Copy link
Contributor

This is a cool tool, good idea!

What are your thoughts on when this check should be run? Suppose new rules show up and start blocking all open PRs on the same set of new lint issues even though no particular PR was the root-cause for the PR to fail. Worse, imagine that multiple people resolve the new lint issues in their PRs in slightly different ways, causing merge conflict headaches for everyone after the first PR to get merged.

I'd almost wish that some periodic automation would create a new independent PR to update lints any time it detects new lint rules created. Then it would be up to others to try to help fix the PR failures before it can get merged in. I know its possible to set up cron-triggered GitHub actions, but I'm not that familiar with automation to create pull requests. Maybe we'd need a separate lints branch of some sort in the main intel/rohd repo which others could open PRs against to fix lint issues? I don't know, open to ideas and thoughts.

@chykon
Copy link
Contributor Author

chykon commented Feb 26, 2023

Indeed, I did not think about possible conflicts in different PRs. It's good that you pointed this out.

I like your suggestion for a way to resolve conflicts and check for new rules.

Reminder: a local check (run_checks.sh) should also not encourage conflicting changes (make checking for new lints optional?).

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.

2 participants