Skip to content

Conversation

matanlurey
Copy link
Contributor

Closes #47090.

@matanlurey matanlurey requested a review from bkonyi July 1, 2025 20:25
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jul 1, 2025
@@ -113,6 +115,8 @@ class UpgradeCommandRunner {
} else {
await runCommandSecondHalf(flutterVersion);
}
final Duration execution = timer.elapsed;
globals.printStatus('Took ${execution.inMinutes} minutes');
Copy link
Contributor

Choose a reason for hiding this comment

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

This will print 'Took 0 minutes' if it takes less than a minute to upgrade. Can we add seconds as well (e.g., 'Completed in 3:15.0')?

Copy link
Member

Choose a reason for hiding this comment

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

If you add this, consider adding a elapsedAsMinutes helper here:

final NumberFormat kSecondsFormat = NumberFormat('0.0');
final NumberFormat kMillisecondsFormat = NumberFormat.decimalPattern();
String getElapsedAsSeconds(Duration duration) {
final double seconds = duration.inMilliseconds / Duration.millisecondsPerSecond;
return '${kSecondsFormat.format(seconds)}s';
}
String getElapsedAsMilliseconds(Duration duration) {
return '${kMillisecondsFormat.format(duration.inMilliseconds)}ms';
}

@matanlurey matanlurey requested review from bkonyi and loic-sharma July 8, 2025 19:32
@matanlurey
Copy link
Contributor Author

Updated based on your feedback @loic-sharma @bkonyi.

I also decided to swap a mysterious bool to a typed class to encapsulate some needed logic.

One thing to consider: this is now a breaking change if a user was using --continue themselves (which we document not to do or rely on, so I don't think this is a big deal). I'm OK with writing a breaking change notice if we think that's relevant, otherwise PTAL.

@matanlurey
Copy link
Contributor Author

I just realized this isn't a totally safe change, someone could be using a Flutter version many back that won't know to pass a date.

I can either:

  1. Print out two sets of times (one for each phase), no sum
  2. Print out a total time if --continue-started-at is passed
  3. Some combination of both

I'm leaning (1); the complexity is already high here, and printing out two separate times, while a little less convenient, still solves the attached issue and we aren't committed to a CLI contract and could always make future changes.

@bkonyi
Copy link
Contributor

bkonyi commented Jul 8, 2025

I don't think this requires a breaking change notice given the explicit warning not to use that flag. This is one of those situations where I wish we didn't have a policy against hiding internal flags completely.

I think as long as the tool can handle the case where --continue isn't given a time, even if that means not printing the upgrade execution time at all for upgrades from versions older than this commit, this should be safe enough.

@matanlurey
Copy link
Contributor Author

@bkonyi Should I assume that's a vote for (2)?

@bkonyi
Copy link
Contributor

bkonyi commented Jul 8, 2025

@bkonyi Should I assume that's a vote for (2)?

If we're going to print this without the user providing --verbose, then yes, I think that's the best solution. Otherwise, users don't know that we're actually running the tool twice as part of the upgrade process and seeing two timings printed may seem strange.

@matanlurey
Copy link
Contributor Author

Updated. PTAL @bkonyi @loic-sharma

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jul 9, 2025
Merged via the queue into flutter:master with commit e212d3d Jul 9, 2025
142 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 10, 2025
flutter/flutter@ac12f66...43657f3

2025-07-10 sokolovskyi.konstantin@gmail.com [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 ybz975218925@gmail.com Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 robert.ancell@canonical.com Refactor compositor classes (flutter/flutter#171414)
2025-07-10 matanlurey@users.noreply.github.com Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 ybz975218925@gmail.com Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 30870216+gaaclarke@users.noreply.github.com Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 matt.boetger@gmail.com Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 43054281+camsim99@users.noreply.github.com Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 matanlurey@users.noreply.github.com Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 40898687+dannyvalentesonos@users.noreply.github.com Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 matanlurey@users.noreply.github.com Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 matanlurey@users.noreply.github.com Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 jessiewong401@gmail.com [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 biggs0125@gmail.com Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 codefu@google.com feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 matanlurey@users.noreply.github.com Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 matanlurey@users.noreply.github.com Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 matanlurey@users.noreply.github.com Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 nt4f04uNd@gmail.com Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 matanlurey@users.noreply.github.com Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 matanlurey@users.noreply.github.com Make `labels` field an array (flutter/flutter#171906)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (#171897)" (flutter/flutter#171910)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (flutter/flutter#171897)
2025-07-09 kjlubick@users.noreply.github.com [skia] Fix flag fiddling for Fuchsia, FreeType, & friends (flutter/flutter#171874)

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 louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 10, 2025
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 28, 2025
vashworth pushed a commit to vashworth/packages that referenced this pull request Jul 30, 2025
…r#9589)

flutter/flutter@ac12f66...43657f3

2025-07-10 sokolovskyi.konstantin@gmail.com [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 ybz975218925@gmail.com Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 robert.ancell@canonical.com Refactor compositor classes (flutter/flutter#171414)
2025-07-10 matanlurey@users.noreply.github.com Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 ybz975218925@gmail.com Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 30870216+gaaclarke@users.noreply.github.com Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 matt.boetger@gmail.com Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 43054281+camsim99@users.noreply.github.com Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 matanlurey@users.noreply.github.com Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 40898687+dannyvalentesonos@users.noreply.github.com Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 matanlurey@users.noreply.github.com Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 matanlurey@users.noreply.github.com Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 jessiewong401@gmail.com [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 biggs0125@gmail.com Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 codefu@google.com feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 matanlurey@users.noreply.github.com Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 matanlurey@users.noreply.github.com Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 matanlurey@users.noreply.github.com Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 nt4f04uNd@gmail.com Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 matanlurey@users.noreply.github.com Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 matanlurey@users.noreply.github.com Make `labels` field an array (flutter/flutter#171906)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (#171897)" (flutter/flutter#171910)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (flutter/flutter#171897)
2025-07-09 kjlubick@users.noreply.github.com [skia] Fix flag fiddling for Fuchsia, FreeType, & friends (flutter/flutter#171874)

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 louisehsu@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
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.

Flutter Upgrade - Add total execution time summary
3 participants