-
Notifications
You must be signed in to change notification settings - Fork 124
ci: update merge conflict bot to trigger on main #970
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
base: main
Are you sure you want to change the base?
ci: update merge conflict bot to trigger on main #970
Conversation
exploreriii
left a comment
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.
Hi @cheese-cakee
Great! Much better, I can work with this - nice one!
One issue I think is that we will never see a failed merge conflict workflow
Right now, you get an X if you have a merge conflict.
By extending it to loop through all issues on push to main, and not letting it fail, we are always going to pass that check, I think.
We want:
✅ If a PR has merge conflicts → its workflow run should fail
✅ If main is updated (push to main) → any PR that now conflicts should also fail in its workflow
✅ No duplicate bot commenting
How can we do that?
|
Thanks for the feedback! Here is my plan to address the three requirements: For individual PRs: I'll make sure the script exits with an error code (exit 1) if a conflict is found. That will give us the red ❌ immediately on the PR. For the loop on main: Since we don't want to fail the workflow running on main itself, I think the best move is to use the GitHub Status API to target the specific PR commit and mark it as failed. For the spam: I'll simply add a check to scan for previous bot comments before posting a new one. Please let me know what you think about this. |
|
excellent ideas! |
|
am i allowed to switch the implementation from the Bash script to actions/github-script |
|
if you are making big changes i would like it to be tested well please 👍 |
|
also if there are any new issues i would love to work on them ! @exploreriii |
|
I am reviewing this currently but we have 4 concurrent bot PRs to review. |
exploreriii
left a comment
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.
Hi @cheese-cakee
Thank you for this - there are many good things here, and might solve the issue!
However you have refactored the logic almost entirely so we cannot go and assume this new bot will work
Therefore, the key thing if you do want to submit this, I want to see you test this on your origin fork
- merge it to main
- create a pull request 1 with a merge conflict - bot should post
- create a pull request 2 without a merge conflict - it should not post
- merge soemthing to main that causes a merge conflcit to pull request 2 - it should now post
- check maximum nubmer of merge conflict posts is 2
Unfortunately, there are quite a few things to test and it is important to do so otherwise we can risk spamming users and sending incorrect messages
Happy for you to go ahead with this issue if you'd like to do that, however, if you do not want to take on the full work (would be quite a bit to test) happy to reassign or even take it on myself.
|
Hi @exploreriii and @MonaaEid , thanks a lot for the detailed feedback ! I definitely want to see this through and implement the refactor + testing myself. I agree that moving the logic to a script and running the 5-step test plan is the right move for stability. If you agree, I will work on these changes and run the full test suite on my fork over the next day or two. I'll ping you here with the results once it's ready. |
|
excellent news! have switched this to draft while its ready to review again |
7fd14a3 to
b7f8b99
Compare
|
Hi @exploreriii I have updated the bot based on the feedback. Changes Made: Moved the helper script to .github/scripts/merge_conflict_helpers.js as requested. Updated the workflow yaml to dynamically locate the script using GITHUB_WORKSPACE, ensuring it works regardless of the runner's path structure. Testing Verification: I have tested this thoroughly on my fork. Test 1(Conflict Detected): Verified that the bot successfully detects a conflict and comments on the PR.
|
|
Hi @cheese-cakee were you able to test if
|
|
changing to draft while it is ready to review again 👍 |
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! From the Hiero Python SDK Team |
|
Hi @exploreriii Yes! I verified this exact scenario in a test PR on my fork. I opened a PR with no conflicts (Bot was silent). I also updated harden-runner to v2.14.0 (the latest release) and just resolved the merge conflicts that popped up with main. Ready for review! |
|
Request review @AntonioCeppellini @MonaaEid @tech0priyanshu if available |
AntonioCeppellini
left a comment
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.
I think this two points needs to be reviewed
|
Hi, this is WorkflowBot.
|
exploreriii
left a comment
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.
Hi @cheese-cakee
Really great you've been able to test it
Could we add one more safety net?
- add workflow dispatch, so we can trigger it manually
- add a dry run variable accepting 0,1,true,false,yes,no
when triggered manually, defeault to dry mode 1
when run on dry mode, it should log to console merge conflict messages instead of comment
Also, you'll need a changelog entry!
|
Hi @cheese-cakee please rebase, the tests are fixed now and there shouldn't be a problem :) |
|
Hi @cheese-cakee I've linked our Rebasing.md file for any assistance you may need. https://github.com/hiero-ledger/hiero-sdk-python/blob/main/docs/sdk_developers/rebasing.md Please reach out if you have any questions! We are always happy to help! |
|
Hello, this is the Office Hour Bot. This is a reminder that the Hiero Python SDK Office Hours are scheduled in approximately 4 hours (14:00 UTC). This session provides an opportunity to ask questions regarding this Pull Request or receive assistance from a maintainer. Details:
Disclaimer: This is an automated reminder. Please verify the schedule here to be notified of any changes. |
|
Hi @cheese-cakee This is looking fantastic! Here is a link to our documentation on CHANGELOG entries Below is an example for the changelog entry: I hope this helps! Please reach out if you have any questions, we are happy to help! :) |
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
Signed-off-by: cheese-cakee <farzanaman99@gmail.com>
af8b9cc to
55122e0
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[commit-verification-bot] To achieve verified status, please read: Remember, you require a GPG key and each commit must be signed with: Thank you for contributing! From the Hiero Python SDK Team |
|
I have completed the rebase.do i need to do anything more like in the changelog? also so sorry for the delay! |
exploreriii
left a comment
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.
Hi @cheese-cakee great to see you back!
Yes, unfortunately your commits are no longer verified:
https://github.com/hiero-ledger/hiero-sdk-python/pull/970/commits
I would recommend soft reverting them, recommiting with appropriate signatures, then force pushing (first create a backup)
|
@cheese-cakee If you need assistance with signing your commits, please let us know :) |
|
Hi @cheese-cakee, This pull request has had no commit activity for 10 days. Are you still working on the issue? please push a commit to keep the PR active or it will be closed due to inactivity. From the Python SDK Team |





Description:
Updates the merge conflict detection workflow to recognize conflicts caused by updates to the
mainbranch. Previously, the bot only notified users if the PR itself was updated, missing conflicts caused by external changes inmain.Changes:
mainupdates.Related issue(s):
Fixes #913
Notes for reviewer:
This addresses the edge case where a PR becomes conflicted due to changes in the base branch, ensuring the author is still notified.
Checklist