docs: clarify pull request review guidelines.

- Move the guidelines from Homebrew/brew Maintainer Guide to
  Maintainer Guidelines as they apply to all repositories.
- Clarify the guidelines to make it clear that the default option
  should be to approve a PR, with or without comments.
- Note in the New Maintainer Checklist that the most important task for
  maintainers is reviewing pull requests.

Co-authored-by: Issy Long <me@issyl0.co.uk>
Co-authored-by: Sam Ford <1584702+samford@users.noreply.github.com>
Co-authored-by: Eric Knibbe <enk3@outlook.com>
This commit is contained in:
Mike McQuaid 2024-04-09 13:57:55 +01:00
parent ef1c54eff9
commit fdcc2c7296
No known key found for this signature in database
3 changed files with 36 additions and 15 deletions

View File

@ -2,16 +2,6 @@
This document describes a few components of the `Homebrew/brew` repository that are useful for maintainers to be aware of, but don't necessarily need to appear in documentation for most users and contributors.
## Reviewing PRs
Using `gh pr checkout NUMBER` is a super easy way to check out a PR branch using the GitHub CLI.
When reviewing a PR, use "comment", "approve", or "request changes" when submitting based on the following guidelines:
- Comment: if the PR isn't quite ready to be merged
- Approve: if you feel that the PR is in a good state to be merged, even if there are non-blocking changes you'd like to be made
- Request changes: if you feel strongly that the PR is likely to cause a problem for users or have another reason to oppose the PR.
## Merging PRs
Merging is done using the standard "Merge" button in the `Homebrew/brew` repository to preserve history and GPG commit signing. The "Squash and Merge" and "Rebase and Merge" buttons are disabled.

View File

@ -15,6 +15,34 @@ All Homebrew maintainers are encouraged to contribute to all parts of the projec
These documents are meant to serve as guiding principles. As a maintainer, you can make a call to either request changes from a contributor or help them out based on their comfort and previous contributions. Remember, as a team we [Prioritise Maintainers Over Users](Maintainers-Avoiding-Burnout.md) to avoid burnout. If you wish to change or discuss any of the guidelines: open a PR to suggest a change.
## Reviewing PRs
When reviewing a PR, use "approve", "approve with comments", "comment" or "request changes" when submitting based on the following guidelines:
- ✅ Approve: if you feel that the PR looks good as is, you can ✅ approve it as-is.
- ✅ Approve with comments: if you have a few questions or comments to be answered and the PR can be merged after they are addressed, you can ✅ approve with comments.
- Please trust that other maintainers will not merge the PR until the comments are addressed rather than e.g. making them wait another 24h to get another review.
- If auto-merge is enabled: you can disable it (add `automerge-skip` on homebrew-core/cask) if you want to ensure the PR is not merged until your comments are at least read.
- 🗣️ Comment: if you need to ask questions before you can provide a ✅ approval but are fine with someone else providing a ✅ approval before you, provide a comment review and ask questions.
- 🚫 Request changes: a last resort.
- When reviewing non-maintainers' PRs: this means "these changes must be made before this PR should be merged by anyone".
Other maintainers can dismiss this review when these changes have been made.
- When reviewing others maintainers' PRs: this is to be avoided whenever possible.
- Save it for "if this PR is merged before I personally have had a chance to ✅ approve it: it seems very likely to cause user-visible problems".
- It may be used by the Project Leader for "this functionality is not acceptable in Homebrew".
In that case: additional code changes should be held off until there's agreement that the functionality is acceptable.
Relatedly:
- The default option should be to ✅ approve, with or without comments.
- Whenever possible, you should try to use the GitHub "suggestion" feature to edit the code how you would like it to be.
If you don't have time or can't be bothered: you probably don't have the time to review the PR properly.
- We're a globally distributed team and this helps us move faster.
- The PR review process is primarily a security measure, not a way to get consensus on the perfect code style before merging.
- It is easy to make changes on a PR after approval, make follow-up PRs to address comments or revert PRs before a tag.
- It is (nearly) impossible to merge a PR without approval.
- Using `gh pr checkout <URL>` is a super easy way to check out a PR branch using the GitHub CLI.
## Mission
Homebrew aims to be the missing package manager for macOS (and Linux). Its primary goal is to be useful to as many people as possible, while remaining maintainable to a professional, high standard by a small group of volunteers. Where possible and sensible, it should seek to use features of macOS to blend in with the macOS and Apple ecosystems. On Linux and Windows, it should seek to be as self-contained as possible.

View File

@ -20,11 +20,14 @@ The Homebrew team and I really appreciate your help on issues, pull requests and
your contributions to Homebrew.
We would like to invite you to have commit access and be a Homebrew maintainer.
If you agree to be a maintainer, you should spend a significant proportion of
the time you are working on Homebrew applying and self-merging widely used
changes (e.g. version updates), triaging, fixing and debugging user-reported
issues, or reviewing user pull requests. You should also be making contributions
to Homebrew at least once per quarter.
If you agree to be a maintainer, you should spend the majority of the time you
are working on Homebrew (in descending order of priority):
- reviewing pull requests (from users and other maintainers)
- triaging, debugging and fixing user-reported issues and applying
- opening PRs for widely used changes (e.g. version updates)
You should also be making contributions to Homebrew at least once per quarter.
You should watch or regularly check Homebrew/brew and/or Homebrew/homebrew-core
and/or Homebrew/homebrew-cask. Let us know which so we can grant you commit