Skip to content

Conversation

@b-luk
Copy link
Contributor

@b-luk b-luk commented Nov 21, 2025

Sorting these entries was suggested by the gemini-code-assist bot in another PR which modified the TESTOWNERS file: #178891 (see comments at #178891 (comment) and #178891 (comment))

This is a follow-up PR to perform the sorting. We don't want to do it in the PR where it was suggested #178891 because sorting the entries is out of scope of that PR's goal.

@b-luk b-luk requested a review from flar November 21, 2025 19:22
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 aims to sort the entries in the TESTOWNERS file. While most of the file has been correctly sorted, I've found a few instances where the lexicographical ordering is incorrect. This seems to happen when one entry's name is a prefix of another. I've left specific comments with suggestions to fix these sorting issues.

Comment on lines 50 to 77
/dev/devicelab/bin/tasks/flutter_gallery__start_up_delayed.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__start_up.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_hybrid.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_with_semantics.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_android__compile.dart @reidbaker @flutter/tool
/dev/devicelab/bin/tasks/flutter_gallery_sksl_warmup__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_lazy__start_up.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_sksl_warmup__transition_perf_e2e.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_sksl_warmup__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_v2_chrome_run_test.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/flutter_gallery_v2_web_compile_test.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/flutter_test_performance.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/fullscreen_textfield_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/fullscreen_textfield_perf__e2e_summary.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/fullscreen_textfield_perf.dart @jtmcdole @flutter/engine
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This block contains several sorting errors. It appears the sorting logic may not be correctly handling cases where one filename is a prefix of another (e.g., ..._perf and ..._perf_e2e). Please correct the order of these entries.

/dev/devicelab/bin/tasks/flutter_gallery__start_up.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__start_up_delayed.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_e2e.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_hybrid.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery__transition_perf_with_semantics.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_android__compile.dart @reidbaker @flutter/tool
/dev/devicelab/bin/tasks/flutter_gallery_lazy__start_up.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_sksl_warmup__transition_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_sksl_warmup__transition_perf_e2e.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/flutter_gallery_v2_chrome_run_test.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/flutter_gallery_v2_web_compile_test.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/flutter_test_performance.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/fullscreen_textfield_perf.dart @jtmcdole @flutter/engine
/dev/devicelab/bin/tasks/fullscreen_textfield_perf__e2e_summary.dart @jtmcdole @flutter/engine

TESTOWNERS Outdated
Comment on lines 262 to 267
/dev/devicelab/bin/tasks/flutter_tool_startup__linux.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup__macos.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup__windows.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup.dart @jensjoha @flutter/tool
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

These flutter_tool_startup entries are not sorted correctly. The entry without a platform suffix should come first.

/dev/devicelab/bin/tasks/flutter_tool_startup.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup__linux.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup__macos.dart @jensjoha @flutter/tool
/dev/devicelab/bin/tasks/flutter_tool_startup__windows.dart @jensjoha @flutter/tool

Comment on lines 308 to 314
/dev/devicelab/bin/tasks/web_benchmarks_ddc_hot_reload.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_skwasm.dart @eyebrowsoffire @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_ddc.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_skwasm_st.dart @eyebrowsoffire @flutter/web
/dev/devicelab/bin/tasks/windows_desktop_impeller.dart @jonahwilliams @flutter/engine
/dev/devicelab/bin/tasks/windows_home_scroll_perf__timeline_summary.dart @jonahwilliams @flutter/engine
/dev/devicelab/bin/tasks/windows_startup_test.dart @loic-sharma @flutter/desktop
/dev/devicelab/bin/tasks/web_benchmarks_skwasm.dart @eyebrowsoffire @flutter/web
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

These web_benchmarks entries are not sorted correctly. It seems to be the same prefix sorting issue as noted in other comments.

/dev/devicelab/bin/tasks/web_benchmarks_ddc.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_ddc_hot_reload.dart @yjbanov @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_skwasm.dart @eyebrowsoffire @flutter/web
/dev/devicelab/bin/tasks/web_benchmarks_skwasm_st.dart @eyebrowsoffire @flutter/web

… sorting.

Because vscode's built in sorting sorts some things in ways we don't want, like periods before underscores.
Copy link
Contributor

@flar flar left a comment

Choose a reason for hiding this comment

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

Please add a description to the PR. I would recommend pointing to the other PR where the review bot is suggesting that this ordering should be maintained.

Copy link
Contributor

@flar flar left a comment

Choose a reason for hiding this comment

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

LGTM

@b-luk b-luk added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Nov 22, 2025
Merged via the queue into flutter:master with commit 6c48c0f Nov 22, 2025
40 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 22, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 22, 2025
flutter/flutter@d438df3...c8cfb2b

2025-11-22 [email protected] Revert "[ios] Dynamic Content Resizing" (flutter/flutter#178958)
2025-11-22 [email protected] Make sure that a SearchAnchor doesn't crash in 0x0 environment (flutter/flutter#177749)
2025-11-22 [email protected] Make sure that a Stepper doesn't crash in 0x0 environment (flutter/flutter#178068)
2025-11-22 [email protected] Remove deprecated `activeColor` in `switch.0.dart` example (flutter/flutter#178293)
2025-11-22 [email protected] Make sure that a UserAccountsDrawerHeader doesn't crash in 0x0 enviro… (flutter/flutter#178516)
2025-11-22 [email protected] Make sure that a TimePickerDialog doesn't crash in 0x0 environment (flutter/flutter#178451)
2025-11-22 [email protected] Make sure that a CupertinoLinearActivityIndicator doesn't crash in 0x… (flutter/flutter#178566)
2025-11-22 [email protected] Make sure that a CupertinoTabBar doesn't crash in 0x0 environment (flutter/flutter#178613)
2025-11-22 [email protected] Make sure that a CupertinoContextMenu doesn't crash in 0x0 environment (flutter/flutter#178722)
2025-11-22 [email protected] Sort entries in TESTOWNERS (flutter/flutter#178939)
2025-11-22 [email protected] [ios] Dynamic Content Resizing (flutter/flutter#177410)
2025-11-21 [email protected] Roll Skia from dc88b21ce7d2 to 3018c3053490 (2 revisions) (flutter/flutter#178947)
2025-11-21 [email protected] Manually roll material_color_utilities (flutter/flutter#170000)
2025-11-21 [email protected] Roll Fuchsia Linux SDK from Y-cMdgKy3d6EnibWR... to 4ul9jvZ7jnDGIjtCD... (flutter/flutter#178934)
2025-11-21 [email protected] Roll Dart SDK from c788b6a7aefd to 5af71c736b0a (1 revision) (flutter/flutter#178932)
2025-11-21 [email protected] Roll Skia from c588bb60d5da to dc88b21ce7d2 (2 revisions) (flutter/flutter#178933)
2025-11-21 [email protected] Update .ci.yaml and TESTOWNERS for arc macrobenchmark tests (flutter/flutter#178891)
2025-11-21 [email protected] Use interactive mode with `devicectl` to redirect stdout (flutter/flutter#178405)
2025-11-21 [email protected] Update .ci.yaml in flutter/flutter to use either macOS 15.5 or macOS … (flutter/flutter#178666)
2025-11-21 [email protected] Roll Skia from d4e9d2873bfd to c588bb60d5da (1 revision) (flutter/flutter#178928)
2025-11-21 [email protected] Roll Packages from b1e2fb0 to e67b6be (7 revisions) (flutter/flutter#178927)

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
@b-luk b-luk deleted the sorttestowners branch December 2, 2025 00:34
mboetger pushed a commit to mboetger/flutter that referenced this pull request Dec 2, 2025
Sorting these entries was suggested by the gemini-code-assist bot in
another PR which modified the TESTOWNERS file: flutter#178891 (see comments at
flutter#178891 (comment)
and
flutter#178891 (comment))

This is a follow-up PR to perform the sorting. We don't want to do it in
the PR where it was suggested flutter#178891 because sorting the entries is out
of scope of that PR's goal.
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
Sorting these entries was suggested by the gemini-code-assist bot in
another PR which modified the TESTOWNERS file: flutter#178891 (see comments at
flutter#178891 (comment)
and
flutter#178891 (comment))

This is a follow-up PR to perform the sorting. We don't want to do it in
the PR where it was suggested flutter#178891 because sorting the entries is out
of scope of that PR's goal.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants