Skip to content

Fix most current canonical score lints #2496

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

Merged
merged 10 commits into from
Feb 2, 2021

Conversation

parlough
Copy link
Member

@parlough parlough commented Jan 29, 2021

You can find the current set here: https://github.com/dart-lang/linter/blob/master/tool/canonical/score.yaml

I disabled non_constant_identifier_names for now due to how our tests are written, but I did fix a few cases of this line.

This PR also deprecates two members.

HTMLBASE_PLACEHOLDER - Replaced with htmlBasePlaceholder which is marked as @internal. This is due to the old constant naming scheme as well as it doesn't seem like this member(or the behavior it enables) should be used externally.

PackageGraph.UninitializedPackageGraph -> Replaced with PackageGraph.uninitialized to fit the appropriate naming style and since the second PackageGraph is unnecessary.

@google-cla google-cla bot added the cla: yes Google CLA check succeeded. label Jan 29, 2021
@parlough parlough changed the title Fix most of current canonical score lints Fix most current canonical score lints Jan 29, 2021
@coveralls
Copy link

coveralls commented Jan 29, 2021

Coverage Status

Coverage decreased (-0.03%) to 91.611% when pulling c7d74cf on parlough:fix/canonical-score-lints into 5b6a529 on dart-lang:master.

@jcollins-g
Copy link
Contributor

What is the motivation for using this particular set of lints? We're mostly going off of pedantic.

@parlough
Copy link
Member Author

parlough commented Jan 30, 2021

You can find some more information here. This is the pub scoring set, which @devoncarew described a derivative of as "a potential proposal for a new recommended rule set for Dart". While the dartdoc package isn't commonly depended on, it will be nice to be consistent with other packages. I don't know much about the specific details as it seems to perhaps be discussed more internally, but I still see it as a win. There is also a recommended set, but that one is larger and can be explored later on.

Even if the final recommendations change, I thought it would be good if we followed them earlier, so future code already follows it and eases the transition.

Copy link
Contributor

@jcollins-g jcollins-g left a comment

Choose a reason for hiding this comment

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

Yes, this does seem to be the way of the future, thanks for getting ahead of it!

Only one change needed; the addition of a hashCode calculation on Class could result in problems.

@jcollins-g jcollins-g merged commit 2e3f4bf into dart-lang:master Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants