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

Allow fine-grained warning control with flags and dartdoc_options.yaml files #1891

Merged
merged 24 commits into from
Jan 11, 2019

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Jan 8, 2019

Fixes #1343, the last long-running dartdoc P1 bug. Fixes #1412 by squelching warnings irrelevant to the package you're documenting. Fixes #1480 by obsoleting it.

The last part of #1343 is providing a way to control warning displays similar to the analyzer and linter, including allowing for a hard-failure option. This PR does that.

Changes:

  • By default, packages not being documented no longer display their warnings or errors. This can be overridden with --allow-non-local-warnings. (further tighten warnings to eliminate false positives #1412)
  • New command line flags --allow-warnings-in-packages, --allow-errors-in-packages, --ignore-warnings-in-packages, and --ignore-errors-in-packages will allow large, multi-package documentation sets to narrow their field of view. Particularly important for Flutter docs.
  • New command line flags --errors, --warnings, and --ignore allow direct control of which warnings are handled in what manner.
  • dartdoc_options.yaml allows errors, warnings, and ignore to be applied to subdirectory trees, so packages can specify what warnings they care about. Command-line arguments supercede these.
  • The --show-warnings command line flag has been removed in favor of the new options. (Put all warnings behind --show-warnings #1480)

Besides the added tests, ran grind compare-flutter-warnings to verify that the new warning code does not accidentally drop warnings on the floor or create new ones:

flutter-docs-current: Generating docs for library web_socket_channel.status from package:web_socket_channel/status.dart...
flutter-docs-current: found 887 warnings and 0 errors
flutter-docs-current: Documented 97 public libraries in 128.9 seconds
flutter-docs-current: Success! Docs generated into /tmp/flutterAPPAHT/dev/docs/doc/api
flutter-docs-current:
flutter-docs-current: Creating a custom index.html in dev/docs/doc/index.html
flutter-docs-current:
flutter-docs-current: Docs ready to go!
flutter-docs-original: Generating docs for library vector_math_geometry from package:vector_math/vector_math_geometry.dart...
flutter-docs-original: Generating docs for library vector_math_lists from package:vector_math/vector_math_lists.dart...
flutter-docs-original:   warning: unresolved doc reference [vectorLength], from vector_math_lists.ScalarListView.ScalarListView: (file:///tmp/pubcacheLWEWFN/hosted/pub.dartlang.org/vector_math-2.0.8/lib/src/vector_math_lists/scalar_list_view.dart:29:3)
flutter-docs-original:   warning: unresolved doc reference [vectorLength], from vector_math_lists.ScalarListView.fromList: (file:///tmp/pubcacheLWEWFN/hosted/pub.dartlang.org/vector_math-2.0.8/lib/src/vector_math_lists/scalar_list_view.dart:38:18)
flutter-docs-original: Generating docs for library vector_math_operations from package:vector_math/vector_math_operations.dart...
flutter-docs-original: Generating docs for library web_socket_channel from package:web_socket_channel/web_socket_channel.dart...
flutter-docs-original: Generating docs for library web_socket_channel.io from package:web_socket_channel/io.dart...
flutter-docs-original:   warning: web_socket_channel.io has no library level documentation comments, from web_socket_channel.io: (file:///tmp/pubcacheLWEWFN/hosted/pub.dartlang.org/web_socket_channel-1.0.9/lib/io.dart:5:9)
flutter-docs-original: Generating docs for library web_socket_channel.status from package:web_socket_channel/status.dart...
flutter-docs-original: found 887 warnings and 0 errors
flutter-docs-original: Documented 97 public libraries in 131.0 seconds
flutter-docs-original: Success! Docs generated into /tmp/dartdoc-comparison-flutterIYMRUF/dev/docs/doc/api
flutter-docs-original:
flutter-docs-original: Creating a custom index.html in dev/docs/doc/index.html
flutter-docs-original:
flutter-docs-original: Docs ready to go!
*** Flutter repo : No difference in warning output from master to HEAD (887 warnings found)

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Jan 8, 2019
@coveralls
Copy link

coveralls commented Jan 8, 2019

Coverage Status

Coverage increased (+0.2%) to 93.697% when pulling 0328e04 on warning-control into ea0392e on master.

Copy link
Collaborator

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

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

This looks great!

The only thing I don't really like is the "warning" terminology itself. It's confusing that something that is an error is referred to as a warning in so many places: there are "error warnings" and "warning warnings". I would have used the word "logging" or "feedback", or something else that doesn't indicate the severity by its name. That could be fixed in another renaming PR though.

@jcollins-g
Copy link
Contributor Author

Yes, I found myself cringing a bit at that too. I'll see what I can do in a followup.

@jcollins-g jcollins-g merged commit 604767f into master Jan 11, 2019
@jcollins-g jcollins-g deleted the warning-control branch January 11, 2019 17:27
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
4 participants