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
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 [email protected] [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 [email protected] Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 [email protected] Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 [email protected] Refactor compositor classes (flutter/flutter#171414)
2025-07-10 [email protected] Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 [email protected] Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 [email protected] Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 [email protected] Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 [email protected] Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 [email protected] Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 [email protected] Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 [email protected] Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 [email protected] Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 [email protected] [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 [email protected] Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 [email protected] feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 [email protected] Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 [email protected] Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 [email protected] Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 [email protected] Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 [email protected] Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 [email protected] 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 [email protected] [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 [email protected],[email protected] 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 [email protected] [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 [email protected] Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 [email protected] Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 [email protected] Refactor compositor classes (flutter/flutter#171414)
2025-07-10 [email protected] Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 [email protected] Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 [email protected] Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 [email protected] Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 [email protected] Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 [email protected] Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 [email protected] Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 [email protected] Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 [email protected] Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 [email protected] [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 [email protected] Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 [email protected] feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 [email protected] Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 [email protected] Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 [email protected] Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 [email protected] Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 [email protected] Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 [email protected] 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 [email protected] [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 [email protected],[email protected] 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
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 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