Skip to content

Add merge conflict github PR check#10812

Merged
mbykhovtsev-ms merged 10 commits into3.0-devfrom
mbykhovtsev/merge-conflict-check
Nov 4, 2024
Merged

Add merge conflict github PR check#10812
mbykhovtsev-ms merged 10 commits into3.0-devfrom
mbykhovtsev/merge-conflict-check

Conversation

@mbykhovtsev-ms
Copy link
Copy Markdown
Contributor

@mbykhovtsev-ms mbykhovtsev-ms commented Oct 22, 2024

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./LICENSES-AND-NOTICES/SPECS/data/licenses.json, ./LICENSES-AND-NOTICES/SPECS/LICENSES-MAP.md, ./LICENSES-AND-NOTICES/SPECS/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge

Summary

Added merge conflict PR check to catch case of cherry-pick tools committing merge conflicts to fix bug https://microsoft.visualstudio.com/OS/_workitems/edit/53883399

Change Log
  • Added merge conflict PR check to catch case of cherry-pick tools committing merge conflicts
Does this affect the toolchain?

NO

Example of simulated committed merge conflict https://github.com/microsoft/azurelinux/actions/runs/11584391919/job/32251428087

@reubeno
Copy link
Copy Markdown
Member

reubeno commented Oct 23, 2024

Naive question: have we explored the native facilities that GitHub provides to ensure that changes aren't in conflict / that the PR branch is up to date with incoming changes in the target branch? Or is there some use case that you're targeting that's not covered by those?

@mbykhovtsev-ms
Copy link
Copy Markdown
Contributor Author

mbykhovtsev-ms commented Oct 23, 2024

Naive question: have we explored the native facilities that GitHub provides to ensure that changes aren't in conflict / that the PR branch is up to date with incoming changes in the target branch? Or is there some use case that you're targeting that's not covered by those?

Yeah, I am targeting in particular edge case of the fast track cherry-pick tool we have, it is the one that would commit merge conflicts and create PRs with them. A different approach might be to make that tool not create PRs and push merge conflicts in the first place, but then it would block the cherry-pick process via tool when any merge conflicts are present that needs to be resolved and will require it be done manually. I believe such edge case is not entirely rare A PR check was a suggestion in this bug by Henry https://microsoft.visualstudio.com/OS/_workitems/edit/53883399.

What other native facilities that Github provides do you suggest?

@reubeno
Copy link
Copy Markdown
Member

reubeno commented Oct 23, 2024

Ah, I didn't realize this was primarily for cherry picks; thanks for the explanation. With that context, I'd agree it's a good idea to prevent unresolved (but already merged) conflicts from getting checked in (i.e., what you're doing here). I also think it's a good idea to pursue how to improve the cherry-picking process to avoid them getting committed in the first place, but agree that's more complicated and separable.

@mbykhovtsev-ms mbykhovtsev-ms marked this pull request as ready for review October 30, 2024 00:04
@mbykhovtsev-ms mbykhovtsev-ms requested a review from a team as a code owner October 30, 2024 00:04
@mbykhovtsev-ms mbykhovtsev-ms merged commit 29f4ae9 into 3.0-dev Nov 4, 2024
@mbykhovtsev-ms mbykhovtsev-ms deleted the mbykhovtsev/merge-conflict-check branch November 4, 2024 22:25
dmcilvaney pushed a commit to dmcilvaney/CBL-Mariner that referenced this pull request Nov 19, 2024
durgajagadeesh pushed a commit to durgajagadeesh/azurelinux_djpalli that referenced this pull request Dec 31, 2024
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.

3 participants