[flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. #4417
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hixie
requested review from
bparrishMines,
ditman,
domesticmouse,
gmackall,
gspencergoog,
jonahwilliams,
reidbaker,
stuartmorgan-g and
tarrinneal
as code owners
github-actions
bot
added
p: camera
p: file_selector
p: flutter_adaptive_scaffold
p: flutter_markdown
p: google_maps_flutter
p: image_picker
p: in_app_purchase
p: local_auth
p: path_provider
p: pigeon
p: shared_preferences
p: standard_message_codec
p: url_launcher
p: video_player
p: webview_flutter
platform-android
labels
Contributor
Author
Hixie
force-pushed
the
excerpt_2
branch
2 times, most recently
from
01dfdcd to
f026326
Compare
reidbaker
approved these changes
Jul 10, 2023
Contributor
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
Approve % stuarts comments.
Collaborator
There was a problem hiding this comment.
You should probably run at least some tests with a MockPlatform(isWindows: true) now that we have custom path parsing/construction logic in the tool itself, to make sure it's behaving as expected on Windows.
Contributor
Author
There was a problem hiding this comment.
added logic to run all the tests with windows as well as the default.
51 tasks
Contributor
Author
Well, the old code using build_runner, the Dart compiler, codegen, stored all the extracts on disk, and had two separate processes for extracting and injecting. The new code does none of that. Not running code is faster than running code. I'm not sure if it makes sense to upstream this because I don't fully understand the website repo's use case so it's hard to say whether this code addresses the same needs. |
Contributor
Author
Added a bit about path-base's default. |
Contributor
Author
|
updated, PTAL. |
tarrinneal
approved these changes
Jul 10, 2023
Contributor
tarrinneal
left a comment
tarrinneal
left a comment
There was a problem hiding this comment.
This is a welcome change, thank you!
Hixie
force-pushed
the
excerpt_2
branch
4 times, most recently
from
248fc71 to
81b0b51
Compare
domesticmouse
approved these changes
Jul 11, 2023
11 tasks
stuartmorgan-g
approved these changes
Jul 11, 2023
Collaborator
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM, thanks!
I saw from your comments in the rfw PR that we may need to iterate more for that specific case, but I think we should land this as-is and then iterate from there in follow-ups since this is already a big quality of life improvement over the current implementation.
Collaborator
There was a problem hiding this comment.
I would vote for removing things we have no intention of ever using (scss and ts).
Contributor
Author
|
I will remove |
…ng on a separate package. * Allows excerpts to come from any package, not just examples. * Fixes a bug in the excerpting logic that was causing a stray `}` to appear in one example. * Removes the need for `build.excerpt.yaml` files. * Remove the dependency on build_runner for excerpts. * Reduces the time to generate the excerpts from about 10 minutes to about 5 seconds. The new logic is not quite backwards compatible; the `path-base` feature now specifies a real path to the actual source directories, rather than a path into the invisible generated `excerpts/` directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per `#docregion` pragma).
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 12, 2023
… in the tool, rather than relying on a separate package. (flutter/packages#4417)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Jul 12, 2023
… in the tool, rather than relying on a separate package. (flutter/packages#4417)
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Jul 12, 2023
flutter/packages@188a846...2508714 2023-07-12 [email protected] ADD appBarBreakpoint (flutter/packages#4434) 2023-07-12 [email protected] Roll Flutter from 65ff3cb to 3ec96a8 (5 revisions) (flutter/packages#4415) 2023-07-12 [email protected] [image_picker] Roll dependancies to avoid error (flutter/packages#4431) 2023-07-12 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump com.android.billingclient:billing from 6.0.0 to 6.0.1 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#4422) 2023-07-12 [email protected] [file_selector] Avoids using path_provider in web example app. (flutter/packages#4445) 2023-07-12 [email protected] [rfw] Add some more documentation for RFW (flutter/packages#4349) 2023-07-12 [email protected] [ci] Enable LUCI legacy analysis (flutter/packages#4435) 2023-07-11 [email protected] [webview_flutter_wkwebview] NSError.toString (flutter/packages#4441) 2023-07-11 [email protected] [ci] Remove unused Chromium setup (flutter/packages#4437) 2023-07-11 [email protected] [flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. (flutter/packages#4417) 2023-07-11 [email protected] [ci] Remove webview_flutter implementation opt outs for custom analysis (flutter/packages#4438) 2023-07-11 [email protected] [palette_generator] Add web support to unit tests (flutter/packages#4440) 2023-07-11 [email protected] [tool] Conditionalize color on `stdout` (flutter/packages#4436) 2023-07-11 [email protected] [go_router_builder] Cleans up builder code. (flutter/packages#4356) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
LouiseHsu
pushed a commit
to LouiseHsu/flutter
that referenced
this pull request
Jul 13, 2023
flutter/packages@188a846...2508714 2023-07-12 [email protected] ADD appBarBreakpoint (flutter/packages#4434) 2023-07-12 [email protected] Roll Flutter from 65ff3cb to 3ec96a8 (5 revisions) (flutter/packages#4415) 2023-07-12 [email protected] [image_picker] Roll dependancies to avoid error (flutter/packages#4431) 2023-07-12 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump com.android.billingclient:billing from 6.0.0 to 6.0.1 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#4422) 2023-07-12 [email protected] [file_selector] Avoids using path_provider in web example app. (flutter/packages#4445) 2023-07-12 [email protected] [rfw] Add some more documentation for RFW (flutter/packages#4349) 2023-07-12 [email protected] [ci] Enable LUCI legacy analysis (flutter/packages#4435) 2023-07-11 [email protected] [webview_flutter_wkwebview] NSError.toString (flutter/packages#4441) 2023-07-11 [email protected] [ci] Remove unused Chromium setup (flutter/packages#4437) 2023-07-11 [email protected] [flutter_plugin_tools] Reimplements the excerpt system inline in the tool, rather than relying on a separate package. (flutter/packages#4417) 2023-07-11 [email protected] [ci] Remove webview_flutter implementation opt outs for custom analysis (flutter/packages#4438) 2023-07-11 [email protected] [palette_generator] Add web support to unit tests (flutter/packages#4440) 2023-07-11 [email protected] [tool] Conditionalize color on `stdout` (flutter/packages#4436) 2023-07-11 [email protected] [go_router_builder] Cleans up builder code. (flutter/packages#4356) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
11 tasks
11 tasks
11 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
}to appear in one example.build.excerpt.yamlfiles.update-excerptswhile IDE is open flutter#107180 (untested).The new logic is not quite backwards compatible; the
path-basefeature now specifies a real path to the actual source directories, rather than a path into the invisible generatedexcerpts/directory with its special structure. Also, a number of features from the previous package that were not actually used in this repository are no longer supported (such as having multiple section names per#docregionpragma).Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).