Skip to content

Conversation

@bkonyi
Copy link
Contributor

@bkonyi bkonyi commented Aug 13, 2025

--dtd-url is only used by widget previews and has no current usage, so this is a safe change.

…review start` option

Also changes the name to `--dtd-uri` for consistency with `dart devtools
--dtd-uri`. `--dtd-url` was only used by widget previews and has no
current usage, so this is a safe change.
@bkonyi bkonyi requested review from DanTup and jyameo August 13, 2025 14:36
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 13, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request moves the --dtd-url flag from a global option to a command-specific option for widget-preview start, and renames it to --dtd-uri for consistency. The changes are mostly correct, but there's a critical issue where the usage of the old global option is not removed, which will cause a build failure. Additionally, a new public constant is missing a doc comment, which is a style guide violation. I've provided comments with details and suggestions.

Copy link
Contributor

@DanTup DanTup left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly moves the --dtd-url flag to be a command-specific option for widget-preview start and renames it to --dtd-uri for consistency. The changes in widget_preview.dart and the corresponding test file are well-implemented. However, there's a critical issue in flutter_command_runner.dart where the global option definition was removed, but its usage in argParser.addOption was not, which will lead to a compilation failure. I've left a comment with details on how to fix this.

@auto-submit
Copy link
Contributor

auto-submit bot commented Aug 13, 2025

autosubmit label was removed for flutter/flutter/173712, because - The status or check suite Windows tool_tests_general has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Mac tool_tests_general has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux tool_tests_general has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Linux analyzer_benchmark has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Aug 13, 2025

autosubmit label was removed for flutter/flutter/173712, because - The status or check suite Linux analyzer_benchmark has failed. Please fix the issues identified (or deflake) before re-applying this label.

  • The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 13, 2025
Merged via the queue into master with commit 1eb2f68 Aug 13, 2025
147 of 148 checks passed
@auto-submit auto-submit bot deleted the widget_preview_dtd_uri branch August 13, 2025 16:51
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 13, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 13, 2025
Roll Flutter from e2a347b14a18 to 34c2a3b158b2 (41 revisions)

flutter/flutter@e2a347b...34c2a3b

2025-08-13 [email protected] [ Tool ] Mark Linux_pixel_7pro linux_chrome_dev_mode as bringup (flutter/flutter#173646)
2025-08-13 [email protected] [ Widget Preview ] Move `--dtd-url` from a global flag to a `widget-preview start` option (flutter/flutter#173712)
2025-08-13 [email protected] Null aware elements clean-ups (flutter/flutter#173074)
2025-08-13 [email protected] Roll Skia from 29e3e1ab7f62 to f7fdda3cd0e6 (3 revisions) (flutter/flutter#173709)
2025-08-13 [email protected] Regular windows win32 engine (flutter/flutter#173424)
2025-08-13 [email protected] Roll Dart SDK from a098cb676fd6 to 73153bdc1459 (1 revision) (flutter/flutter#173708)
2025-08-13 [email protected] Roll Fuchsia Linux SDK from vgv-rTf7i9PfcDq2Y... to I1TfNmsqTp7t3rO8e... (flutter/flutter#173690)
2025-08-13 [email protected] Roll Skia from 1170405c30cf to 29e3e1ab7f62 (2 revisions) (flutter/flutter#173689)
2025-08-13 [email protected] Roll Dart SDK from e2b7aec7333e to a098cb676fd6 (4 revisions) (flutter/flutter#173683)
2025-08-13 [email protected] Roll Skia from d06fdf03c6a1 to 1170405c30cf (4 revisions) (flutter/flutter#173681)
2025-08-13 [email protected] Roll Skia from 9ed4b4e53db2 to d06fdf03c6a1 (11 revisions) (flutter/flutter#173661)
2025-08-12 [email protected] Fix GTK redraw call being called from non-GTK thread. (flutter/flutter#173602)
2025-08-12 [email protected] [Impeller] Apply Y coordinate scaling when sampling from the destination texture in framebuffer advanced blends (flutter/flutter#173639)
2025-08-12 [email protected] Fix directional focus in nested scrollables with different axis (flutter/flutter#172875)
2025-08-12 [email protected] [ios][tools]do not log "bonjour not found" at all (unless verbose) (flutter/flutter#173569)
2025-08-12 [email protected] Remove jetifier usages  (flutter/flutter#173548)
2025-08-12 [email protected] [ Tool ] Fix run_linux_chrome_dev_mode (flutter/flutter#173647)
2025-08-12 [email protected] [ios] Update iOS code signing CIPD instruction command (flutter/flutter#171173)
2025-08-12 [email protected] Reapply "Make device debuggable if useDwdsWebSocketConnection is true … (#173551)" (flutter/flutter#173628)
2025-08-12 [email protected] Roll Clang to 8c7a2ce01a77c96028fe2c8566f65c45ad9408d3 (flutter/flutter#173429)
2025-08-12 [email protected] [web] Fallback to CanvasKit when WebGL is not available (flutter/flutter#173629)
2025-08-12 [email protected] Roll Packages from a114ac2 to 08a9b2c (3 revisions) (flutter/flutter#173625)
2025-08-12 [email protected] [ Tool ] Fix crash from possible DDS startup race (flutter/flutter#173362)
2025-08-12 [email protected] Roll Skia from a2936eff2179 to 9ed4b4e53db2 (3 revisions) (flutter/flutter#173611)
2025-08-12 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4 to 5 in the all-github-actions group (flutter/flutter#173606)
2025-08-12 [email protected] Roll Dart SDK from c5fe48aee60d to e2b7aec7333e (1 revision) (flutter/flutter#173604)
2025-08-12 [email protected] Roll Skia from 44bb9d908ee4 to a2936eff2179 (21 revisions) (flutter/flutter#173603)
2025-08-12 [email protected] Fix the issue of over-scrolling in SliverMainAxisGroup with a PinnedHeaderSliver. (flutter/flutter#173349)
2025-08-12 [email protected] Roll Dart SDK from b2a23936f968 to c5fe48aee60d (2 revisions) (flutter/flutter#173596)
2025-08-11 [email protected] Update CanRenderTiledTexture unit tests (flutter/flutter#173553)
2025-08-11 [email protected] Update integration test for iOS deployment workflows (flutter/flutter#173566)
2025-08-11 [email protected] Enables vulkan for PowerVR B-Series (flutter/flutter#173561)
2025-08-11 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reapply "Make device debuggable if useDwdsWebSocketConnection is true … (#173551)" (#173568)" (flutter/flutter#173587)
2025-08-11 [email protected] Roll Dart SDK from 8e882349fcab to b2a23936f968 (2 revisions) (flutter/flutter#173571)
2025-08-11 [email protected] Reapply "Make device debuggable if useDwdsWebSocketConnection is true … (#173551)" (flutter/flutter#173568)
2025-08-11 [email protected] Update CI iOS tests (flutter/flutter#173563)
2025-08-11 [email protected] Roll Packages from 34948d1 to a114ac2 (4 revisions) (flutter/flutter#173556)
2025-08-11 [email protected] Roll Fuchsia Linux SDK from HclTm0V8hgSpfqmtG... to vgv-rTf7i9PfcDq2Y... (flutter/flutter#173505)
2025-08-11 [email protected] Roll Dart SDK from 6a7ae1ffd1c9 to 8e882349fcab (2 revisions) (flutter/flutter#173499)
2025-08-11 [email protected] Update `ExpansibleController` in `ExpansionTile` `didUpdateWidget` (flutter/flutter#173175)
2025-08-11 [email protected] add format cmd to tools instruction (flutter/flutter#173428)

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
Please CC [email protected],[email protected] on the revert to ensure that a human
...
SydneyBao pushed a commit to SydneyBao/flutter that referenced this pull request Aug 14, 2025
…review start` option (flutter#173712)

`--dtd-url` is only used by widget previews and has no current usage, so
this is a safe change.
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…review start` option (flutter#173712)

`--dtd-url` is only used by widget previews and has no current usage, so
this is a safe change.
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
…review start` option (flutter#173712)

`--dtd-url` is only used by widget previews and has no current usage, so
this is a safe change.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
…review start` option (flutter#173712)

`--dtd-url` is only used by widget previews and has no current usage, so
this is a safe change.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…review start` option (flutter#173712)

`--dtd-url` is only used by widget previews and has no current usage, so
this is a safe change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants