Skip to content

Fix lints and warnings accumulated in dartdoc #1635

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 5 commits into from
Mar 16, 2018
Merged

Fix lints and warnings accumulated in dartdoc #1635

merged 5 commits into from
Mar 16, 2018

Conversation

jcollins-g
Copy link
Contributor

Most are due to recent library changes, but the rest have just been building up in dartdoc for a long time. This cleans them up.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Mar 15, 2018
Copy link
Member

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

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

Need to update the SDK constraint to >=2.0.0-dev.9 – when the new constants were introduced...

@@ -7,7 +7,7 @@ homepage: https://github.com/dart-lang/dartdoc
# For development, recommend 1.25.0-dev.0.0 or higher to allow
# dartanalyzer to work on dartdoc itself.
environment:
sdk: '>=1.23.0-dev.11.5 <2.0.0'
sdk: '>=2.0.0-dev.9.0 <3.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member

Choose a reason for hiding this comment

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

This constraint is handy to know. Thanks!

Copy link
Member

@pq pq left a comment

Choose a reason for hiding this comment

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

👍

@@ -7,7 +7,7 @@ homepage: https://github.com/dart-lang/dartdoc
# For development, recommend 1.25.0-dev.0.0 or higher to allow
# dartanalyzer to work on dartdoc itself.
environment:
sdk: '>=1.23.0-dev.11.5 <2.0.0'
sdk: '>=2.0.0-dev.9.0 <3.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

This constraint is handy to know. Thanks!

@@ -224,6 +221,7 @@ typedef int LotsAndLotsOfParameters(so, many, parameters, it, should, wrap,

/// This class is cool!
class Cool {
// ignore: missing_return
Cool returnCool() {}
Copy link
Member

Choose a reason for hiding this comment

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

Aside: why not just return null? In general, I'm curious why all the ignores and not just fixes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test package is intended to be a bit strange, using outdated, error-prone and/or obsolete constructions in some places. So I don't want to accidentally make the test package too "clean".

I shouldn't be doing that anywhere outside testing/ though.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Thanks!

@jcollins-g jcollins-g merged commit f1808c9 into master Mar 16, 2018
@jcollins-g jcollins-g deleted the fix-lints branch March 16, 2018 18:06
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.

4 participants