-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[tool] Remove use of FETCH_HEAD #10357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[tool] Remove use of FETCH_HEAD #10357
Conversation
Changes the default branch used to find merge bases from `FETCH_HEAD` to `main`. We were arranging for it to be `main` in CI, and locally it was an unpredictable value, so just hard-coding to `main` is likely to give better behavior locally with no change to CI behavior. Fixes flutter/flutter#176738
There was a problem hiding this 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 changes the default branch for finding merge bases from FETCH_HEAD to main in the repository's tooling. The changes are applied to GitVersionFinder, with corresponding updates to command-line help text and tests. A git fetch command in a CI script is also removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nonblocking nit. Do you want to use some other environment variable in these scripts like PACKAGES_MAIN then set the value if it is not already set? That way there is a way to compare against something else you control if you wanted to. I ask because it is probably easier to do in this pr than it would be to find all these location later.
There's already a flag ( (In general I don't like using env variables in the tooling because it creates spooky action at a distance; there used to be more use of it, and it was really hard to figure out what things in CI were actually doing because you'd have to find some magic env variable set somewhere other than where most of the CI script was.) |
* 'main' of https://github.com/CaoGiaHieu-dev/packages: Update packages/go_router/CHANGELOG.md [tool] Remove use of FETCH_HEAD (flutter#10357) Roll Flutter from 027f2e410241 to e5d5c01850f2 (73 revisions) (flutter#10362) [camera_platform_interface] Adds support for video stabilization to camera_platform_interface (flutter#10337) [google_maps_flutter] Raise `MapUsedAfterWidgetDisposedError` when map controller used after map disposed (flutter#9242) [pigeon] Replace containsKey with contains in Kotlin generator (flutter#10274) [video_player] Remove `package` in example `AndroidManifest.xml` file (flutter#10245) [flutter_svg] Fixes typo of `allowDrawingOutsideViewBox` in doc comments. (flutter#10256) [in_app_purchase] Remove use of Pigeon's Dart test generator (flutter#10328) [dependabot]: Bump com.squareup.okhttp3:okhttp from 5.1.0 to 5.3.0 in /packages/espresso/android (flutter#10348) Roll Flutter from 6f8abdd77820 to 027f2e410241 (26 revisions) (flutter#10335) [google_sign_in] Remove use of OCMock (flutter#10290) [interactive_media_ads] Pin iOS dependency maximum (flutter#10349)
Changes the default branch used to find merge bases from
FETCH_HEADtomain. We were arranging for it to bemainin CI, and locally it was an unpredictable value, so just hard-coding tomainis likely to give better behavior locally with no change to CI behavior.Fixes flutter/flutter#176738