Allow branches to be excluded in tide queries #7456
Conversation
k8s-ci-robot
added
kind/feature
stevekuznetsov
force-pushed
the
skuznets/tide-branches
branch
from
51a1d9b to
f4ced77
Compare
prow/tide/tide.go
Outdated
| desc = fmt.Sprintf(" Merging to branch %s is forbidden.", pr.BaseRef.Name) | ||
| } | ||
| } | ||
| diff = 1 |
There was a problem hiding this comment.
Isn't this really one more diff vs a single diff?
There was a problem hiding this comment.
Looks like this return value is consumed by expectedStatus(). Can we document the meaning of this return value?
There was a problem hiding this comment.
I branched this out so we only consider it when other diffs don't apply, so no in this case there are no other diffs and it is just 1. This means that if there's a query against which everything passes except for the branch issue, it will be prioritized. Unclear if that is the right direction /cc @cjwagner
There was a problem hiding this comment.
We should check that the branch is acceptable first instead of last. If it is unacceptable we should prioritize that message so that users don't try to meet the other merge requirements only to discover that PRs can't merge to that branch at all.
There was a problem hiding this comment.
Any appetite for documenting the meaning of diff count?
There was a problem hiding this comment.
You know me, always ravenous for documentation
🍔 🍟 🍺
|
|
prow/tide/tide.go
Outdated
| desc = fmt.Sprintf(" Merging to branch %s is forbidden.", pr.BaseRef.Name) | ||
| } | ||
| } | ||
| diff = 1 |
There was a problem hiding this comment.
We want to increase diff even if we don't end up showing a message about that requirement in the status description. See this review: #7454 (review)
|
I'll add docs and update the diff-count mechanism after we merge #7454 and I rebase |
|
LGTM but Michalis and Cole are doing the bulk of the heavy lifting here. |
k8s-ci-robot
added
the
needs-rebase
stevekuznetsov
force-pushed
the
skuznets/tide-branches
branch
from
f4ced77 to
cc84ed3
Compare
k8s-ci-robot
removed
the
needs-rebase
In order to support code freeze on a branch, the queries used to gather pull requests for the tide pool need too allow negative base ref searches. The status posted by `tide` also expose this behavior on PRs against those branches that are otherwise good to merge. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
stevekuznetsov
force-pushed
the
skuznets/tide-branches
branch
from
cc84ed3 to
e9343e1
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cjwagner, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
In order to support code freeze on a branch, the queries used to gather
pull requests for the tide pool need too allow negative base ref
searches. The status posted by
tidealso expose this behavior on PRsagainst those branches that are otherwise good to merge.
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
Fixes #7450
/area prow
/kind feature
/cc @Kargakis @fejta
/assign @cjwagner @BenTheElder