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.

Sign up for GitHub

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

Jump to bottom

Add some additional lints #1569

Merged
merged 22 commits into from
Aug 23, 2022
Merged

Add some additional lints #1569

merged 22 commits into from
Aug 23, 2022

Conversation

TimWhiting
Copy link
Collaborator

@TimWhiting TimWhiting commented Aug 18, 2022 •
edited
Loading

Migrated a few deprecated analyzer apis. Handle dependencies for constructor tearoffs and assignment of ref to objects using cascades.

Still not handling escape of ref via functions. Will work on that next.

@TimWhiting TimWhiting marked this pull request as draft August 18, 2022 06:22
@rrousselGit
Copy link
Owner

Nice! The CI seem to be failing though

@TimWhiting TimWhiting changed the title handle a few more cases Add some additional lints Aug 19, 2022
@TimWhiting
Copy link
Collaborator Author

I've added hot-reload support to the linter for local testing / debugging, a lot easier than using the custom_lint.log file & exceptions / print statements, because you can use breakpoints and the debugger. Happy to contribute that to custom_lint, but a little more generalized. WDYT?

@TimWhiting TimWhiting changed the base branch from linter to master August 20, 2022 02:53
@rrousselGit
Copy link
Owner

I've added hot-reload support to the linter for local testing / debugging, a lot easier than using the custom_lint.log file & exceptions / print statements, because you can use breakpoints and the debugger. Happy to contribute that to custom_lint, but a little more generalized. WDYT?

Sure. Although I honestly don't quite understand what you meant

@TimWhiting
Copy link
Collaborator Author

This is the relevant code and generalization to support fast debugging
for any custom lint plugin.

invertase/dart_custom_lint#26 (comment)

@TimWhiting TimWhiting marked this pull request as ready for review August 23, 2022 04:14
@TimWhiting
Copy link
Collaborator Author

TimWhiting commented Aug 23, 2022 •
edited
Loading

Let's squash and merge this. I'm continuing work on it, but I want to let others contribute, and this is in a good state as it is. The CI build is failing for the same reason the build is failing on the master branch.

rrousselGit
rrousselGit reviewed Aug 23, 2022
View reviewed changes
examples/counter/analysis_options.yaml
@@ -0,0 +1,3 @@
analyzer:
# plugins:
Copy link
Owner

Choose a reason for hiding this comment

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

Why add custom_lint to disable it?

Copy link
Owner

Choose a reason for hiding this comment

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

Ah wait since you're based on my branch, that could be me.

rrousselGit
rrousselGit reviewed Aug 23, 2022
View reviewed changes
packages/riverpod/example/lib/models.dart
@@ -45,6 +45,9 @@ class Repository {
'hash': hash,
},
);
if (result.data == null) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why this change?

@rrousselGit rrousselGit merged commit f7bdaed into rrousselGit:master Aug 23, 2022
@TimWhiting TimWhiting deleted the linter branch September 24, 2022 05:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rrousselGit rrousselGit rrousselGit left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@TimWhiting @rrousselGit