diff --git a/docs/Homebrew-brew-Maintainer-Guide.md b/docs/Homebrew-brew-Maintainer-Guide.md index 84a9415987..e0a622d1ba 100644 --- a/docs/Homebrew-brew-Maintainer-Guide.md +++ b/docs/Homebrew-brew-Maintainer-Guide.md @@ -7,10 +7,13 @@ be aware of, but don't necessarily need to appear in documentation for most user Using `gh pr checkout NUMBER` is a super easy way to check out a PR branch using the GitHub CLI. -When reviewing, choose the "comment" type for reviews when the PR isn't quite ready to be merged. -Use the "approve" type when 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. Use the "request changes" type if you feel strongly -that the PR is likely to cause a problem for users or have a genuine reason to oppose the PR. +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 a genuine reason to oppose the PR. ## Merging PRs @@ -20,13 +23,13 @@ PRs must meet the following conditions to be merged: - Have at least one maintainer (or `BrewTestBot`) approval. See [below](#automatic-approvals) for more details about how `BrewTestBot` approves PRs. -- Passing CI. This is a _mandatory_ step. PRs with failing CI should _never_ be merged. +- Have passing CI. This is a _mandatory_ step. PRs with failing CI should _never_ be merged. See [below](#ci) for more information about `Homebrew/brew` CI. If possible, PRs should also have: -- Linear commit history (i.e. no merge commits in PR branches) -- GPG-signed commits (see the private `ops` repository for instructions on setting this up) +- Linear commit history (i.e. no merge commits in PR branches). +- GPG-signed commits (see the private `ops` repository for instructions on setting this up). ## Automatic approvals @@ -56,27 +59,28 @@ There are many checks that run on every PR. The following is a quick list of the - `Vendor Gems / vendor-gems`: This is skipped except for dependabot PRs. It updates the RBI files to match any new/changed dependencies. See [Type Checking With Sorbet](Typechecking.md) for more information about RBI files and typechecking. -- `Triage / review`: This controls whether the PR has been open for long enough or not. +- `Triage / review`: This verifies that the PR has been open for long enough. See [above](#automatic-approvals) for more information about automatic approvals. - `codecov/patch` and `codecov/project`: These show the Codecov report for the PR. - See [below](#brew-tests-and-codecov) for more info about Codecov -- `CI / vendored gems (Linux)`: This checks whether there was a change to the venered gems on Linux that needs to be + See [below](#brew-tests-and-codecov) for more info about Codecov. +- `CI / vendored gems (Linux)`: This checks whether there was a change to the vendored gems on Linux that needs to be committed to the PR branch. -- `CI / test default formula (Linux)`: This runs `brew test-bot` Linux to ensure it still works as expected. +- `CI / test default formula (Linux)`: This runs `brew test-bot` on Linux to ensure it still works as expected. - `CI / syntax`: This is run first to check whether the PR passes `brew style` and `brew typecheck`. If this job fails the following jobs will not run. - `CI / tap syntax (Linux)`: This runs `brew style` and `brew audit` on all official taps (note that although this has Linux in its name, it does check `Homebrew/homebrew-core`, `Homebrew/linuxbrew-core` and all cask repos). - `CI / docker`: This builds and deploys a new Homebrew docker image. -- `CI / test everything (macOS)`: This runs `brew tests` on macOS +- `CI / test everything (macOS)`: This runs `brew tests` on macOS. - `CI / tests (no-compatibility mode)`, `CI / tests (generic OS)` and `CI / tests (Linux)`: These run `brew tests` with various options on Linux. ### `brew tests` and Codecov A coverage report is generated by Codecov for every PR, and its results are shown as CI jobs. -Additionally, annotations will appear in the "Files changed" where lines of code have been +These reports are publicly viewable on [Homebrew/brew's Codecov page](https://app.codecov.io/gh/Homebrew/brew). +Additionally, annotations will appear in the PR's "Files changed" tab where lines of code have been added that aren't being hit by `brew tests`. If the Codecov job fails, that's a sign that some more tests should be added to test the functionality being added in the PR. diff --git a/docs/Homebrew-homebrew-cask-Maintainer-Guide.md b/docs/Homebrew-homebrew-cask-Maintainer-Guide.md index d5f5970141..712a4a0f21 100644 --- a/docs/Homebrew-homebrew-cask-Maintainer-Guide.md +++ b/docs/Homebrew-homebrew-cask-Maintainer-Guide.md @@ -12,7 +12,7 @@ This guide applies to all four of the cask repositories: ## Common Situations -Here is a list of the most common situations that arise in PRs and how to handle them: +Here is a list of the most common situations that arise in cask PRs and how to handle them: - The `version` and `sha256` both change (keeping the same format): Merge. - Only the `sha256` changes: Merge unless the version needs to be updated as well. @@ -25,18 +25,19 @@ Here is a list of the most common situations that arise in PRs and how to handle If in doubt, ask another cask maintainer on GitHub or Slack. -Note that unlike in formulae, casks do not consider the `sha256` stanza as meaningful security measure as maintainers cannot realistically check them for authenticity. Casks download from upstream; if a malicious actor compromised a URL, they could just as well compromise a version and make it look like an update. +Note that unlike formulae, casks do not consider the `sha256` stanza to be a meaningful security measure +as maintainers cannot realistically check them for authenticity. Casks download from upstream; if a malicious +actor compromised a URL, they could just as well compromise a version and make it look like an update. ## Merging ### Approvals -PRs in the cask repositories should must have at least one approval -from a user with write access before they can be merged. - -Ideally, this approval will come from another maintainer. If necessary, however, a maintainer -can self-approve one of their PRs using the `self-approve` GitHub Actions workflow to satisfy -this requirement. To trigger a self-approval, navigate to the +Most PRs in the cask repositories are simple version bumps that don't necessarily need +another maintainer's approval. However, GitHub will not allow a PR to be merged without +at least one approving review. To bypass this requirement if necessary, a maintainer +can self-approve one of their PRs using the `self-approve` GitHub Actions workflow to +satisfy this requirement. To trigger a self-approval, navigate to the ["Self-approve a Pull Request" section of the Actions tab](https://github.com/Homebrew/homebrew-cask/actions/workflows/self-approve.yml), click on "Run workflow", enter the PR number and click "Run workflow". diff --git a/docs/Homebrew-homebrew-core-Merge-Checklist.md b/docs/Homebrew-homebrew-core-Merge-Checklist.md index bba579ac5e..b3c858c7fe 100644 --- a/docs/Homebrew-homebrew-core-Merge-Checklist.md +++ b/docs/Homebrew-homebrew-core-Merge-Checklist.md @@ -72,7 +72,7 @@ Only one maintainer is necessary to approve and merge the addition of a new or u Here are guidelines about when to use squash & merge versus rebase & merge. These options should only be used with PRs where bottles are not needed or affected. -| | PR modified a single formula | PR modifies multiple formulae | +| | PR modifies a single formula | PR modifies multiple formulae | |---|---|---| | **Commits look good** | rebase & merge _or_ squash & merge | rebase & merge | | **Commits need work** | squash & merge | manually merge using the command line | @@ -117,13 +117,13 @@ Formulae that: should not be removed from Homebrew. The exception to this rule are [versioned formulae](Versions.md) for which there are higher standards of usage and a maximum number of versions for a given formula. For more information about deprecating, disabling and removing formulae, see the -[Deprecating, Disabling, and Removing Formulae page](Deprecating-Disabling-and-Removing-Formulae.md) +[Deprecating, Disabling, and Removing Formulae page](Deprecating-Disabling-and-Removing-Formulae.md). ## Detailed merge checklist The following checklist is intended to help maintainers decide on whether to merge, request changes or close a PR. It also brings more -transparency for contributors in addition to +transparency for contributors in addition to the [Acceptable Formulae](Acceptable-Formulae.md) requirements. - previously opened active PRs, as we would like to be fair to contributors who came first diff --git a/docs/Maintainer-Guidelines.md b/docs/Maintainer-Guidelines.md index d67448bbea..b74b08bf5c 100644 --- a/docs/Maintainer-Guidelines.md +++ b/docs/Maintainer-Guidelines.md @@ -80,7 +80,7 @@ PRs that are an "enhancement" to existing functionality i.e. not a fix to an ope If a maintainer is on holiday/vacation/sick during this time and leaves comments after they are back: please treat post-merge PR comments and feedback as you would left within the time period and follow-up with another PR to address their requests (if agreed). -The vast majority of `Homebrew/homebrew-core` PRs are bug fixes or version bumps so can be self-merged once CI has completed. +The vast majority of `Homebrew/homebrew-core` PRs are bug fixes or version bumps which can be self-merged once CI has completed. ## Communication diff --git a/docs/README.md b/docs/README.md index f062d09d5d..df46165646 100644 --- a/docs/README.md +++ b/docs/README.md @@ -61,10 +61,10 @@ - [Maintainers: Avoiding Burnout](Maintainers-Avoiding-Burnout.md) - [Maintainer Guidelines](Maintainer-Guidelines.md) -- [`brew` Maintainer Guide](Brew-Maintainer-Guide.md) -- [Core Maintainer Guide](Homebrew-homebrew-core-Merge-Checklist.md) -- [Linux Maintainer Guide](Linux-Maintainer-Guide.md) -- [Cask Maintainer Guide](Cask-Maintainer-Guide.md) +- [Homebrew/brew Maintainer Guide](Homebrew-brew-Maintainer-Guide.md) +- [Homebrew/homebrew-core Maintainer Guide](Homebrew-homebrew-core-Merge-Checklist.md) +- [Homebrew/linuxbrew-core Maintainer Guide](Homebrew-linuxbrew-core-Maintainer-Guide.md) +- [Homebrew/homebrew-cask Maintainer Guide](Homebrew-homebrew-cask-Maintainer-Guide.md) - [Brew Test Bot For Maintainers](Brew-Test-Bot-For-Core-Contributors.md) - [Common Issues for Maintainers](Common-Issues-for-Core-Contributors.md)