[tool] Rename batch change override label#12263
Conversation
(again) This was given a more descriptive lbale in flutter#11835, but it appears based on failures when attempting to generate the PR that there is an undocumented length limit of 50 characters for label names, and `override: skip-batch-release-repo-check-` uses 40 of them, which causes problems when adding the package name. This renames to `override: batch-`, which is less descriptive, but should work around the length limit. Longer term, we may need to revisit having the package name in the label to avoid edge case issues with long package names.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the validator and associated tests to replace the override label 'override: skip-batch-release-repo-check-' with 'override: batch-'. Feedback on this change notes that the new label format may still exceed GitHub's 50-character limit for packages with long names, and suggests considering a shorter prefix or a generic label to prevent potential issues.
| // checks. | ||
| final bool shouldSkipBatchReleaseRepoCheck = _prLabels.contains( | ||
| 'override: skip-batch-release-repo-check-${pubspec.name}', | ||
| 'override: batch-${pubspec.name}', |
There was a problem hiding this comment.
While renaming the label to override: batch-${pubspec.name} reduces the length, it may still exceed GitHub's 50-character label limit for packages with long names.
For example:
interactive_media_ads_platform_interface(40 chars) +override: batch-(16 chars) = 56 characters.google_maps_flutter_platform_interface(38 chars) +override: batch-(16 chars) = 54 characters.shared_preferences_platform_interface(37 chars) +override: batch-(16 chars) = 53 characters.
Consider using a shorter prefix (e.g., batch-${pubspec.name}) or a generic label like override: batch-release that applies to all packages in the PR to avoid hitting this limit.
There was a problem hiding this comment.
For posterity: we can't remove override: without changing Cocoon, because there is Cocoon-level plumbing that carries over just override: * labels to a context where the CI tooling can see them.
There was a problem hiding this comment.
You could also truncate the package name to the first 44 characters and hope we never end up with a collision.
There was a problem hiding this comment.
I think if we have length issues again in practice we should probably just switch to a single static label rather than a per-package label. Things would have to go pretty seriously wrong for changes to be happening in the wrong package, and I would think the human review step would catch that.
| // checks. | ||
| final bool shouldSkipBatchReleaseRepoCheck = _prLabels.contains( | ||
| 'override: skip-batch-release-repo-check-${pubspec.name}', | ||
| 'override: batch-${pubspec.name}', |
There was a problem hiding this comment.
You could also truncate the package name to the first 44 characters and hope we never end up with a collision.
justinmc
added
the
autosubmit
(again) This was given a more descriptive lbale in flutter#11835, but it appears based on failures when attempting to generate the PR that there is an undocumented length limit of 50 characters for label names, and `override: skip-batch-release-repo-check-` uses 40 of them, which causes problems when adding the package name. This renames to `override: batch-`, which is less descriptive, but should work around the length limit. Longer term, we may need to revisit having the package name in the label to avoid edge case issues with long package names.
2 participants
(again)
This was given a more descriptive lbale in
#11835, but it appears based on failures when attempting to generate the PR that there is an undocumented length limit of 50 characters for label names, and
override: skip-batch-release-repo-check-uses 40 of them, which causes problems when adding the package name.This renames to
override: batch-, which is less descriptive, but should work around the length limit.Longer term, we may need to revisit having the package name in the label to avoid edge case issues with long package names.