migrate template defaults to use package:pedantic for lints #594

pq · 2019-01-16T18:55:00Z

Migrates templates to use the package:pedantic lint rule set.

~~I started with just one options file to keep word-smithing / bike-shedding in one place. When we're happy with it, I'll update the others before proposing to land.~~

Ready for review.

Additional context: https://github.com/dart-lang/linter/issues/1365.

Fixes #593.

/cc @kwalrath @kevmoo @chalin

kwalrath

lgtm, so far

kevmoo · 2019-01-17T20:03:52Z

I'm on the fence about using the actual pedantic import, etc. I think it's a bit too much magic for brand new packages.

I'd rather we copy-paste the lints in the file w/ a link to the pedantic repo.

pq · 2019-01-17T21:26:04Z

I hear you. On the other hand:

maybe it's good to nudge folks to use includes (because, you know, reuse, hygiene, yada, yada)
if pana uses the latest pedantic lints to score packages (and I think it should), there will be skew and package scores could dip without the possible early warning
I'm hoping we can update the Flutter templates to use includes so this will be more consistent w/ them
an include makes it easier to eyeball if a package is "pedantic"

pq · 2019-01-18T18:29:22Z

@kevmoo, @kwalrath: I took a stab at inlining the rules with some explanation. Needless to say it could use word-smithing but I wonder how the approach sits with you.

kevmoo · 2019-01-18T18:43:44Z

Thanks. There is an upside to depending on pkg:pedantic, but then we have the discussion about "do we pin the version or set a range". If we pin it, then we have to explain why we pin it. If we set a range, then 'pub upgrade' may add new lint warnings, which is weird. I think leaving it to the user to add the dependency is goodness

pq · 2019-01-18T19:04:25Z

@kevmoo: that's my thinking too. I totally punted on pinning and that's probably OK. Should we at least include the pedantic version we're deriving from in the comment? Maybe something like:

# Google-default lint rules as defined in package:pedantic (v1.4.0).
# See: https://github.com/dart-lang/pedantic for details and the latest
# ruleset.

Back to pinning, maybe we could add a breadcrumb in the pubspec:

dev_dependencies:
#   include the latest pedantic rules 
#   pedantic: ^1.0.0
#   alternatively, pin to a specific version, if you want to ensure that newly added rules can't break us
#   pedantic: 1.4.0

/fyi @davidmorgan.

kevmoo · 2019-01-18T19:05:13Z

Great idea!

…

On Fri, Jan 18, 2019 at 11:04 AM Phil Quitslund ***@***.***> wrote: @kevmoo <https://github.com/kevmoo>: that's my thinking too. I totally punted on pinning and that's probably OK. Should we at least include the pedantic version we're deriving from in the comment? Maybe something like: # Google-default lint rules as defined in package:pedantic (v1.4.0). # See: https://github.com/dart-lang/pedantic for details and the latest # ruleset. Back to pinning, maybe we could add a breadcrumb in the pubspec: dev_dependencies: # include the latest pedantic rules # pedantic: ^1.0.0 # alternatively, pin to a specific version, if you want to ensure that newly added rules can't break us # pedantic: 1.4.0 /fyi @davidmorgan <https://github.com/davidmorgan>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#594 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCigFlO2K6l_y3pqjN3yUHMEhqE2GFks5vEhq7gaJpZM4aDuUM> .

pq · 2019-01-18T19:10:22Z

Great. I updated one template:

https://github.com/dart-lang/stagehand/pull/594/files#diff-79e948527db3596d41d525bb56aae0cb
https://github.com/dart-lang/stagehand/pull/594/files#diff-067bb172fdf806e8624bc04f8885b458

If folks approve, I'll go ahead and update the rest.

Thanks!

/cc @kwalrath @chalin

chalin · 2019-01-18T19:40:22Z

My vote would be for @pq's original proposal: just import pedantic.

In this way newbies get the default set of lints out-of-the-box; a list that is continually updated and curated by expert Dart users (mainly based, I assume on internal projects). Power users will know how to twiddle things :) to their liking, including if/when to pin a package to a version or not.

I don't like the non-DRY suggestion of having a comment like "Google-default lint rules as defined in package:pedantic (v1.4.0)" and then duplicating the pedantic list (from one specific point in time). That is just more maintenance for us down the line.

My 2 cents :)

devoncarew · 2019-01-18T20:42:35Z

My vote would be for @pq's original proposal: just import pedantic.

Yeah, this does keep things nice and simple.

kevmoo · 2019-01-18T21:18:57Z

VS having these out of sync w/ pedantic, I'll take the import. Proceed!

…

On Fri, Jan 18, 2019 at 12:42 PM Devon Carew ***@***.***> wrote: My vote would be for @pq <https://github.com/pq>'s original proposal: just import pedantic. Yeah, this does keep things nice and simple. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#594 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABCiuXmZsJk_uHe8svuzl3h4ZaVRR-uks5vEjG9gaJpZM4aDuUM> .

pq · 2019-01-18T23:35:47Z

OK! Updated to the first proposal and maybe ready to go?

PTAL. Word-smithing welcome.

kevmoo

LGTM

chalin

Overall LGTM. I'll let @kwalrath have the final word w.r.t. word-smithing :)

davidmorgan · 2019-01-21T09:52:50Z

Looks good, thanks!

pq · 2019-01-22T18:32:57Z

@kwalrath : any thoughts on wording (or otherwise)?

kwalrath · 2019-01-22T21:42:33Z

Looking now...

kwalrath

Once you remove the unneeded semicolon before the URL (in all pubspec copies), LGTM.

kwalrath · 2019-01-22T21:43:46Z

templates/analysis_options.yaml

@@ -1,14 +1,14 @@
+# Defines a default set of lint rules enforced for
+# projects at Google. For details and rationale,
+# see: https://github.com/dart-lang/pedantic#enabled-lints.


see: -> see

Fixed. Thanks!

migrate template defaults to use package:pedantic for lints

b94dc91

pq mentioned this pull request Jan 16, 2019

[meta] rationalize/align/document different lint rule sets dart-lang/sdk#57873

Closed

11 tasks

kwalrath reviewed Jan 16, 2019

View reviewed changes

kwalrath requested review from kevmoo and chalin January 16, 2019 20:21

inline rules

3394497

removed pedantic dep

5f2db09

version details

6bee683

=> pedantic include

9d26432

pq requested a review from devoncarew January 18, 2019 23:35

kevmoo approved these changes Jan 18, 2019

View reviewed changes

pq added 2 commits January 18, 2019 23:45

regenerated compressed templates

bacd38c

update test

33120c0

chalin approved these changes Jan 19, 2019

View reviewed changes

kwalrath reviewed Jan 22, 2019

View reviewed changes

dropped colon; regenerated templates

b6d0ff4

kwalrath approved these changes Jan 22, 2019

View reviewed changes

kwalrath merged commit dad2cde into master Jan 22, 2019

kwalrath deleted the pedantic branch January 22, 2019 22:08

kwalrath mentioned this pull request Jan 22, 2019

Show pedantic in analysis-options dart-lang/site-www#1322

Closed

pq mentioned this pull request Jan 23, 2019

Consider using pedantic as a baseline for linting? dart-lang/pana#459

Closed

kwalrath mentioned this pull request Jan 29, 2019

Update analysis options for pedantic dart-lang/site-www#1328

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate template defaults to use package:pedantic for lints #594

migrate template defaults to use package:pedantic for lints #594

pq commented Jan 16, 2019 •

edited

Loading

kwalrath left a comment

kevmoo commented Jan 17, 2019

pq commented Jan 17, 2019

pq commented Jan 18, 2019

kevmoo commented Jan 18, 2019

pq commented Jan 18, 2019

kevmoo commented Jan 18, 2019 via email

pq commented Jan 18, 2019

chalin commented Jan 18, 2019

devoncarew commented Jan 18, 2019

kevmoo commented Jan 18, 2019 via email

pq commented Jan 18, 2019

kevmoo left a comment

chalin left a comment

davidmorgan commented Jan 21, 2019

pq commented Jan 22, 2019

kwalrath commented Jan 22, 2019

kwalrath left a comment

kwalrath Jan 22, 2019

pq Jan 22, 2019

migrate template defaults to use package:pedantic for lints #594

migrate template defaults to use package:pedantic for lints #594

Conversation

pq commented Jan 16, 2019 • edited Loading

kwalrath left a comment

Choose a reason for hiding this comment

kevmoo commented Jan 17, 2019

pq commented Jan 17, 2019

pq commented Jan 18, 2019

kevmoo commented Jan 18, 2019

pq commented Jan 18, 2019

kevmoo commented Jan 18, 2019 via email

pq commented Jan 18, 2019

chalin commented Jan 18, 2019

devoncarew commented Jan 18, 2019

kevmoo commented Jan 18, 2019 via email

pq commented Jan 18, 2019

kevmoo left a comment

Choose a reason for hiding this comment

chalin left a comment

Choose a reason for hiding this comment

davidmorgan commented Jan 21, 2019

pq commented Jan 22, 2019

kwalrath commented Jan 22, 2019

kwalrath left a comment

Choose a reason for hiding this comment

kwalrath Jan 22, 2019

Choose a reason for hiding this comment

pq Jan 22, 2019

Choose a reason for hiding this comment

pq commented Jan 16, 2019 •

edited

Loading