-
Notifications
You must be signed in to change notification settings - Fork 175
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
auto-merge label not set on docs cherrypicks #4140
Comments
Correct, adding the label there would apply to all repos, which we don't want. I believe we just need to create another cherry-picker for the istio.io repo only which adds the additional label (and remove istio.io from the current one).. |
Is this something that is still needed? |
It would be nice, yes! |
I am not sure it's safe to allow arbitrary people to decide arbitrary
content should go to arbitrary branches without review
…On Wed, Jun 28, 2023, 6:06 AM Craig Box ***@***.***> wrote:
It would be nice, yes!
—
Reply to this email directly, view it on GitHub
<#4140 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEYGXJZDFU7ELITK7EUPCDXNQT5DANCNFSM52FF3NHQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
Actually thinking about it more - this is extremely unsafe, we cannot do
this without some additional restrictions.
…On Wed, Jun 28, 2023, 6:10 AM John Howard ***@***.***> wrote:
I am not sure it's safe to allow arbitrary people to decide arbitrary
content should go to arbitrary branches without review
On Wed, Jun 28, 2023, 6:06 AM Craig Box ***@***.***> wrote:
> It would be nice, yes!
>
> —
> Reply to this email directly, view it on GitHub
> <#4140 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAEYGXJZDFU7ELITK7EUPCDXNQT5DANCNFSM52FF3NHQ>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
|
I agree for nearly all repos, we do not want auto merge on cherry-picks for the reasons John stated. My original statement was that the behavior today is org wide, and thus we shouldn't make the change. Craig is asking about istio.io only, and for that I'm less particular about the auto-merge on the cherry-pick since it's typically doc related and for the most part, the approval is pretty automatic anyway. A reason not to do it for istio.io, would be the typical auto-merge would be into the current production branch (the istio.io website) and that's very visible very quickly after the merge. |
The reason to do it is because the reality is we write most of the docs for 1.2x when 1.2x is out, rather than doing it all beforehand and freezing it like we would in a perfect world. Another possible reason not to do it would be that anyone could set I would like to get to a world where we only have the docs for supported Istio releases online... |
https://github.com/istio/istio.io/#publishing-content-immediately says that when the
cherrypick/release-1.14
label is set on an istio/istio.io PR, it should be merged automatically.The label itself says "Set this label on a PR to auto-merge it to the release-1.14 branch'.
The auto-merge label isn't set on the PRs created by the cherrypicker (https://github.com/istio/test-infra/blob/master/prow/cluster/cherrypicker_deployment.yaml#L20-L29).
@ericvn assumes this is because we only want this behavior on the docs repo, and if the
auto-merge
label was set here, it would apply project-wide?I can't talk for the other repos, but as the docs repo needs to cherrypick almost every change, it would be a great quality of life improvement to change this here.
The text was updated successfully, but these errors were encountered: