Fix merge queue CI #11797
Merged
Fix merge queue CI #11797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jade-guiton-dd
added
the
Skip Changelog
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11797 +/- ##
=======================================
Coverage 91.43% 91.43%
=======================================
Files 447 447
Lines 23745 23745
=======================================
Hits 21712 21712
Misses 1657 1657
Partials 376 376 ☔ View full report in Codecov by Sentry. |
evan-bradley
approved these changes
Dec 4, 2024
mowies
approved these changes
Dec 5, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 5, 2024
#### Description In a previous PR (#11797), I removed the `merge_group` trigger from the `changelog` and `api-compatibility` CI checks, which don't work properly in merge queues. However, `changelog` is a required check for the `main` branch, which means Github will wait for the check status to be reported even though the job never started, eventually resulting in a timeout and rejection from the queue. ([see this note in the docs](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#status-checks-with-github-actions-and-a-merge-queue)) This PR adds the `merge_group` trigger back in `changelog`, but uses an `if` to skip it outside of pull requests (which should count as success for the purposes of the merge queue).
4 tasks
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this pull request
Dec 19, 2024
#### Description This PR removes the `merge_group` trigger from the two CI checks that failed in merge queues (but not in PRs). See tracking issue for details. The reasoning for simply disabling them instead of modifying them to work in merge queues is three-fold: - For `changelog` to be meaningful, it needs to only run for PRs with no `[chore]` title tag or `Skip changelog` label, but it seems there is no simple way to access this data in merge queue workflows unfortunately. (A possible complicated way may be to parse the ref (PR branch) name and query the title and labels in a script with the `gh` command.) - `api-compatibility` is never meaningful because it seems to have been broken for a long time (see open-telemetry#7443) - Both `changelog` and `api-compatibility` are checks on the _set of changes_ made by a PR, not on the resulting state of the codebase. As such, their result should not meaningfully change when merged against an old main vs. a newer main, making running them in the merge queue pointless. #### Link to tracking issue Fixes open-telemetry#11790 This should let us try [enabling the merge queue](open-telemetry/community#2469) again. #### Testing Testing merge queue checks is unfortunately quite difficult, but since we're only disabling checks I don't foresee any issues.
HongChenTW
pushed a commit
to HongChenTW/opentelemetry-collector
that referenced
this pull request
Dec 19, 2024
…1810) #### Description In a previous PR (open-telemetry#11797), I removed the `merge_group` trigger from the `changelog` and `api-compatibility` CI checks, which don't work properly in merge queues. However, `changelog` is a required check for the `main` branch, which means Github will wait for the check status to be reported even though the job never started, eventually resulting in a timeout and rejection from the queue. ([see this note in the docs](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks#status-checks-with-github-actions-and-a-merge-queue)) This PR adds the `merge_group` trigger back in `changelog`, but uses an `if` to skip it outside of pull requests (which should count as success for the purposes of the merge queue).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR removes the
merge_grouptrigger from the two CI checks that failed in merge queues (but not in PRs). See tracking issue for details.The reasoning for simply disabling them instead of modifying them to work in merge queues is three-fold:
changelogto be meaningful, it needs to only run for PRs with no[chore]title tag orSkip changeloglabel, but it seems there is no simple way to access this data in merge queue workflows unfortunately. (A possible complicated way may be to parse the ref (PR branch) name and query the title and labels in a script with theghcommand.)api-compatibilityis never meaningful because it seems to have been broken for a long time (see [CI] 'Inform Incompatible PRs / Check-Compatibility' doesn't work #7443)changelogandapi-compatibilityare checks on the set of changes made by a PR, not on the resulting state of the codebase. As such, their result should not meaningfully change when merged against an old main vs. a newer main, making running them in the merge queue pointless.Link to tracking issue
Fixes #11790
This should let us try enabling the merge queue again.
Testing
Testing merge queue checks is unfortunately quite difficult, but since we're only disabling checks I don't foresee any issues.