Skip to content
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.

Sign up for GitHub

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

Jump to bottom

change(merge): Require 2 reviews for PRs with an extra-reviews label #7158

Merged
merged 5 commits into from
Jul 10, 2023
Merged

change(merge): Require 2 reviews for PRs with an extra-reviews label #7158

merged 5 commits into from
Jul 10, 2023

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Jul 5, 2023 •
edited
Loading

Motivation

This PR makes sure we do two reviews on PRs with the extra-reviews label.

Closes #5996

Specifications

https://docs.mergify.com/examples/#using-labels-to-enable-disable-merge

Solution

  • Require 2 reviews if the extra-reviews label is on a PR
  • Automatically add an extra-reviews label with release drafter to PRs with titles with:
    • remov*
    • deprecat*
    • crash*
    • destr*
    • unsafe*
  • Clean up the auto-labelling in release-drafter.yml

Review

This would be good to have for PR #7053.

Reviewer Checklist

  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
    • Does it change concurrent code, unsafe code, or consensus rules?
  • How do you know it works? Does it have tests?

@teor2345 teor2345 added A-devops Area: Pipelines, CI/CD and Dockerfiles P-Medium ⚡ C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG labels Jul 5, 2023
@teor2345 teor2345 requested a review from a team as a code owner July 5, 2023 20:58
@teor2345 teor2345 self-assigned this Jul 5, 2023
@teor2345 teor2345 requested a review from a team as a code owner July 5, 2023 20:58
@teor2345 teor2345 requested review from gustavovalverde and oxarbitrage and removed request for a team July 5, 2023 20:58
teor2345
teor2345 commented Jul 5, 2023
View reviewed changes
.github/release-drafter.yml Outdated Show resolved Hide resolved
@github-actions github-actions bot added the C-enhancement Category: This is an improvement label Jul 5, 2023
@teor2345 teor2345 mentioned this pull request Jul 5, 2023
Merged
6 tasks
@teor2345 teor2345 mentioned this pull request Jul 7, 2023
Merged
6 tasks
@teor2345
Copy link
Contributor Author

teor2345 commented Jul 9, 2023

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Jul 9, 2023

update

✅ Branch has been successfully updated

This was referenced Jul 9, 2023
Closed
Merged
Merged
gustavovalverde
gustavovalverde approved these changes Jul 10, 2023
View reviewed changes
Copy link
Member

@gustavovalverde gustavovalverde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! And most of us agreed using this approach and label wording

@teor2345
Copy link
Contributor Author

This PR contains a mergify config, so it needs a manual merge.

@teor2345 teor2345 merged commit d3cc91e into main Jul 10, 2023
@teor2345 teor2345 deleted the require-extra-reviews branch July 10, 2023 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gustavovalverde gustavovalverde gustavovalverde approved these changes

@oxarbitrage oxarbitrage Awaiting requested review from oxarbitrage oxarbitrage is a code owner automatically assigned from ZcashFoundation/general-rust-reviewers

Assignees

@teor2345 teor2345

Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Require two reviewers for complex or risky code changes
2 participants
@teor2345 @gustavovalverde