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

Adding flutter_lints #227

Merged
merged 5 commits into from
Jun 8, 2021
Merged

Conversation

domesticmouse
Copy link
Contributor

Hey @matthew-carroll,

I tried adding flutter_lints, but the tests are failing. However, the reason is unmigrated code in the spikes directory, so I'm unsure what you want to do about that.

brett

@matthew-carroll matthew-carroll self-requested a review June 4, 2021 04:17
@matthew-carroll
Copy link
Contributor

I'll look through this. It's probably time to delete the spikes.

One thing I'm noticing is a possible line-length difference. I have mine set to 120 in the IDE. Do you know if there's a way to set that in the lint rules? I think 80 is too short.

@domesticmouse
Copy link
Contributor Author

I'll reformat for 120

@domesticmouse
Copy link
Contributor Author

Looks like configuring Dartfmt's line length is a non-starter: dart-lang/dart_style#918

I'd suggest adding a check to your Github Actions that checks if a dart format with line length of 120 changes things. Happy to add it to this PR if ya want.

@matthew-carroll
Copy link
Contributor

I'd suggest adding a check to your Github Actions that checks if a dart format with line length of 120 changes things. Happy to add it to this PR if ya want.

Sure, if you know where/how to add that then I'd say go for it. Thanks.

@domesticmouse
Copy link
Contributor Author

Pulled the 120 char line length change into a separate PR: #229

@matthew-carroll
Copy link
Contributor

@domesticmouse it looks like a number of tests broke with this PR. Any idea why?

@domesticmouse
Copy link
Contributor Author

Looks like I constified lists that shouldn't have been consted. I'll re-run locally and debug.

@domesticmouse
Copy link
Contributor Author

Tests now pass locally. Hopefully it should also pass in CI.

@domesticmouse
Copy link
Contributor Author

Non-netlify tests pass. No idea why the netlify tests are failing.

Halp?

@matthew-carroll
Copy link
Contributor

The website build failures are unrelated to this, we can ignore them. I'll merge this in.

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