chore(workflow): make major version releases safer #8725
Conversation
| exit 1 | ||
| else | ||
| echo "Success: Approval requirement of $REQUIRED_APPROVALS met." | ||
| fi No newline at end of file |
There was a problem hiding this comment.
it would be better to combine this check with the existing Major Version check in our existing "Release Checks" workflow file, instead of creating a new workflow
There was a problem hiding this comment.
that was my original plan. However, I needed to add a new trigger for this workflow "on pull-request-review: submit" so it would run whenever there is a new approval on the PR. There isn't a way to add a trigger per job. I can only add it per workflow. I didn't want to add a new trigger to existing jobs that don't need it.
There was a problem hiding this comment.
Then I think we can rely on the existing check to be sufficient
There was a problem hiding this comment.
Can you give me some guidance on how I would combine the check for 2 yoshi-php approvers with the existing unexpected-major-version-check? Also, there are other jobs in the existing Release Checks workflow that do not need the "on pull-request-review: submit" trigger. So even if I combined the 2 approver check workflow into the existing unexpected-major-version-check, I would need to separate it out into a new workflow OR add unnecessary trigger to other existing jobs.
Also, I thought we talked about implementing the 2 approvers minimum workflow for major version releases. I want to understand what you mean by existing check being sufficient.
There was a problem hiding this comment.
I just mean that the 2 approvers minimum isn't strictly necessary, and would rather omit it in favor of relying on the existing check, than introduce this additional workflow.
There was a problem hiding this comment.
The 2 approvers minimum check would only be required for major version releases. It would prevent issues like last week’s release. Considering how much impact accidental major version releases can have on our clients, a single dev alone should not be able to execute it.
There was a problem hiding this comment.
I think since we already have a release check for it below, which forces devs to confirm that they indeed want to release a major version, and because this PR improves the error check to output a very clear warning, that's sufficient to prevent this from happening. IMO two approvals for the release PR is overkill (as is the additional workflow)
b/459567217