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

Add lints for v2.0 #72

Merged
merged 2 commits into from
Mar 17, 2022
Merged

Add lints for v2.0 #72

merged 2 commits into from
Mar 17, 2022

Conversation

goderbauer
Copy link
Contributor

See https://github.com/dart-lang/lints/projects/1.

Fixes #64
Fixes #42
Fixes #23
Fixes #71
Fixes #61
Fixes #66
Fixes #68
Fixes #67
Fixes #70
Fixes #58

@goderbauer
Copy link
Contributor Author

/cc @mit @pq @srawlins

pubspec.yaml Outdated

environment:
sdk: '>=2.12.0 <3.0.0'
sdk: '>=2.17.0-0 <3.0.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to verify that this is the lower bound that we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since nobody has raised any concerns about this in the email thread, I am assuming this constraint is good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a non-stable lower bound is OK. It has the added benefit that we can publish prior to 2.17 stable going out, and pub.dev will show the package as a "pre-stable release" until the exact moment 2.17 stable launches.

But rather than using -0 we should perhaps use the build number for the lowest release that supports all the lints that we're including here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to set this to 2.17.0-206.0.dev, which is the lowest version that contains all autofixes for the lints we are enabling.

For reference, the last autofix was added in dart-lang/sdk@0d130c3. This version has not rolled into Flutter yet, but should be in the next few hours.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@@ -8,13 +8,15 @@ linter:
- camel_case_extensions
- camel_case_types
- curly_braces_in_flow_control_structures
- depend_on_referenced_packages
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto fix for this one is still under discussion, see #42.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are going to ship this without an autofix.

@goderbauer
Copy link
Contributor Author

I believe we are ready to submit this PR. @pq or @mit-mit can you review it?

@goderbauer goderbauer requested a review from mit-mit March 15, 2022 17:13
@pq
Copy link
Member

pq commented Mar 16, 2022

🚀

@goderbauer
Copy link
Contributor Author

I am just waiting on 2.17.0-206.0.dev to successfully roll into Flutter before submitting this PR. This is the PR to watch for the roll: flutter/flutter#100270

@goderbauer goderbauer merged commit 8294e56 into dart-lang:main Mar 17, 2022
@goderbauer goderbauer deleted the lints2 branch March 17, 2022 21:15
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment