-
Notifications
You must be signed in to change notification settings - Fork 248
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
REQUEST: Repository maintenance on opentelemetry-collector #2469
Comments
done, will leave open until you're able to confirm it's working |
Spoke offline with Trask, disabled it for now since we found a couple of issues: open-telemetry/opentelemetry-collector#11781 (comment) |
Just to confirm, are the issues related to my PR in any way? |
@VihasMakwana No, not at all, I just used your PR as a test :) |
#### 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 #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 #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.
@trask We believe we have fixed the issues and would like to try again. Could you enable the merge queue again? |
@mx-psi Enabled the merge queue again with the following settings: |
For the record, there was yet another issue (the |
Thanks, @jade-guiton-dd! @mx-psi Please confirm once everything is working as expected so we can close the issue. |
Seems to be working as expected, closing :) |
#### 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.
Affected Repository
https://github.com/open-telemetry/opentelemetry-collector
Requested changes
Enable the Require merge queue setting on the branch protection rules for the main branch with "squash" merge method
Purpose
Same as #2457
Expected Duration
Permanently
Repository Maintainers
The text was updated successfully, but these errors were encountered: