Ignore builds on other branches #148
Conversation
|
@FPtje This is a fix for the multiple-comment issue you and Bert reported yesterday. Can you please review when you have the time? GitHub's documentation isn't super clear of what's the criteria for including the branch in the branches list: https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#status. Here's what it says:
If that's true in the most straightforward interpretation, there is no way to actually identify the "canonical" build for a sha in a branch! In this PR I assume the list only includes the branches for which the build refers to (this would make more sense). In the case of Semaphore, there can be multiple builds for the same commit sha if they are on different branches. My plan is to see in practice what happens after we deploy this change:
In case of "2.", I will have to prepare a second PR where different URLs for the same SHA are simply ignored. This will make Hoff "internally inconsistent" but at least we will not get duplicate comments. Either way, we will get a clarification of what exactly GitHub does because the webhook PR branches will be logged. |
|
I'm a little short on time. @frank-channable could you review? Thanks! |
|
@frank-channable Thanks for the review. @OpsBotPrime mergeee |
|
Pull request approved for merge by @rudymatela, rebasing now. |
Approved-by: rudymatela Auto-deploy: false
|
Rebased as 88a0ec6, waiting for CI … |
|
CI job started. |
@OpsBotPrime You do accept trailing alphabetic characters... Interesting... |
|
CI job started. |
|
@OpsBotPrime test. |
|
That was not a valid command. |
|
@OpsBotPrime merge |
|
CI job started. |
This makes sense. The reply to the merge command when it is in progress is the current status: "CI job started". 👍 |
This reverts commit 88a0ec6, reversing changes made to 78cfbfb. The changes in PR #148 did not fix what it was intended to fix (#147). At the same time they introduced a bug in extreme cases: when more than 10 branches point to the same commit, we may loose a build status change notification on the right branch. We could maintain these changes partially to include logging what branches githubs lists, but I don't think the maintaining costs justify this.
|
Message from the future: this merge has been reverted in faf04c9. |
closes: #147
If there are two builds on different branches but with the same hash, GitHub reports these at least twice! (Once per branch.) This makes it so that we only consider build status changes on the actual relevant testing branch. There's a detailed explanation here.