Test-infra: prow: are we (ab)suing /hold?

[stevekuznetsov]

Looks like the new paradigm in this repo is to /lgtm but /hold for someone else to look at it. What we are trying to convey is that the change looks good to us but that merging requires a larger quorum. Was this the intent of /hold? Do we want to think up a mechanism that is more explicitly achieving this workflow? Are we hacking the current plugins to get what we want?

/cc @kargakis @cjwagner @BenTheElder

[stevekuznetsov]

/area prow
/kind question

[BenTheElder]

This paradigm definitely seems clear to me in a some cases besides finding quorum: EG adding a repo to tide tends to mean lgtm + hold with the intention that someone will cancel the hold once the upstream repo is ready.

I suppose we could just LGTM when we want to "approve" and wait for quorum. I do also think /hold being generally used to delay merging seems fine. We could have some kind of multiple-approvers-required system instead for the quorum case, but I'm not convinced this is worth the effort at the moment.

[mattfarina]

Here's an example I have for using hold. I'm an approver for all of the charts repo. There are times where a chart change LGTM but I would like the chart owner to look at it. In that capacity I'm acting as a reviewers but I implicitly approve. So, I hold for the chart owner to approve.

[spiffxp]

I like /hold as a catch-all for socially enforced workflows. I always try and drop a comment under my /hold to explain why I'm holding it, eg: what I'm waiting for, who I'm cool with canceling the hold, etc.

If we need something that can automatically be enforced, maybe there's something other than /hold that we should be automating.

[fejta]

I would like PRs to have /hold by default. Otherwise I think /hold is working out exactly as I intended -- a generic catch-all for socially enforced workflows.

We have other silliness like WIP, release-note-required, but I like how /hold and /hold cancel are human, easy, flexible.

[stevekuznetsov]

So what I'm hearing is that even in the specific case where we often use /hold to wait for review quorum it's working fine as a socially enforced process and not worth codifying. SGTM

/close