-
Notifications
You must be signed in to change notification settings - Fork 30
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
Tighten up the gen_docs.dart
script.
#187
Conversation
Move some functionality into helper functions, to make the overall logic easier to read. Extracted constants, added comments. Make the script work when called from any directory (finds package root relative to `Platform.script`, instead of assuming current working directory is the root.) Remove dependency on `package:collection` which was only used for one function, in code that was restructured. General functionality should still be the same (the generated output and JSON cache haven't changed).
(I fell over the script during something else and wanted to change a little thing. Then it snowballed.) I'm not entirely sure I understand the point of the "validate" mode, which checks that the cached JSON and generated Markdown are in sync, which they should be when both are written in the same runs of the tool. It would make more sense to check that the cached JSON is up-to-date with the remote source-of-truth, because if not, we should update both local files, or to generate and write the markdown if it's not in sync with the cached JSON, which can happen if modifying the tool itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make the script work when called from any directory (finds package root relative to Platform.script, instead of assuming current working directory is the root.)
I tend to enforce calling from a known location (the package root, the repo root, ...) and fast fail if run from elsewhere; but no particular objections to this technique.
@@ -5,98 +5,208 @@ | |||
import 'dart:convert'; | |||
import 'dart:io'; | |||
|
|||
import 'package:collection/collection.dart'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Yeah, the way I look at it, we want:
Probably that means that in the 'verify only' mode, we can skip downloading a new json file and just use what's in the repo. |
I'll consider a separate change to the logic, where the "verify" mode becomes "rebuild without downloading", so it actually saves the new generated documentation. (And tells you if there are any changes.) Landing this for now. |
Revisions updated by `dart tools/rev_sdk_deps.dart`. async (https://github.com/dart-lang/async/compare/09cba7f..77a25d7): 77a25d7 2024-05-15 Sarah Zakarias Add `topics` to `pubspec.yaml` (dart-archive/async#274) dartdoc (https://github.com/dart-lang/dartdoc/compare/2e706be..476d5cc): 476d5cc8 2024-05-16 Sam Rawlins Refactor PackageGraph._tagReexportsFor and document (dart-lang/dartdoc#3772) 24658cca 2024-05-14 Sam Rawlins Privatize CommentReferenceParser._codeRef (dart-lang/dartdoc#3771) dff86ed9 2024-05-14 Sam Rawlins Bump to 8.0.9 (dart-lang/dartdoc#3770) http (https://github.com/dart-lang/http/compare/4722e03..76deb75): 76deb75 2024-05-16 Hossein Yousefi [cronet_http] Upgrade jni to 0.9.2 and publish 1.2.1 (dart-lang/http#1198) ec55561 2024-05-15 Brian Quinlan [cronet] Use the same host and Android emulator architecture. (dart-lang/http#1201) lints (https://github.com/dart-lang/lints/compare/f0205c1..b254c7e): b254c7e 2024-05-16 Devon Carew Update README.md (dart-lang/lints#189) 5fef508 2024-05-13 Lasse R.H. Nielsen Tighten up the `gen_docs.dart` script. (dart-lang/lints#187) test (https://github.com/dart-lang/test/compare/84d2a2b..2464ad5): 2464ad5c 2024-05-16 Sarah Zakarias Add `topics` to package "test" `pubspec.yaml` (dart-lang/test#2230) 6540a360 2024-05-15 dependabot[bot] Bump the github-actions group across 1 directory with 3 updates (dart-lang/test#2229) 4b6f029c 2024-05-15 dependabot[bot] Bump dart_flutter_team_lints from 2.1.1 to ^3.0.0 in all packages (dart-lang/test#2228) webdev (https://github.com/dart-lang/webdev/compare/d46cf50..fc32eb6): fc32eb69 2024-05-14 Elliott Brooks Collect log message count and log at the end (dart-lang/webdev#2430) 99abc535 2024-05-14 Elliott Brooks Wait for a `resume` event to run the `main()` method after a page refresh (dart-lang/webdev#2431) Change-Id: Iee28bacfc028c92e4b59d95f0c7e61f8282d2968 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/366980 Auto-Submit: Devon Carew <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]>
Move some functionality into helper functions, to make the overall logic easier to read. Extracted constants, added comments. Make the script work when called from any directory (finds package root relative to `Platform.script`, instead of assuming current working directory is the root.) Remove dependency on `package:collection` which was only used for one function, in code that was restructured. General functionality should still be the same (the generated output and JSON cache haven't changed).
Move some functionality into helper functions, to make the overall logic easier to read. Extracted constants, added comments.
Make the script work when called from any directory (finds package root relative to
Platform.script
, instead of assuming current working directory is the root.)Remove dependency on
package:collection
which was only used for one function, in code that was restructured.General functionality should still be the same (the generated output and JSON cache haven't changed).