docs: add brew maintainer guide

This commit is contained in:
Rylan Polster 2021-05-13 02:30:01 -04:00
parent 068292cfdc
commit e0230330d0
No known key found for this signature in database
GPG Key ID: 46A744940CFF4D64
4 changed files with 241 additions and 137 deletions

View File

@ -0,0 +1,98 @@
# `brew` Maintainer Guide
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 repo 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.
## Merging PRs
Merging should be 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.
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.
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)
## Automatic approvals
To ensure that each PR has the opportunity to be seen and reviewed by any other maintainers who wish
to take a look, all PRs require an approval before they can be merged. However, not every PR is
reviewed by another maintainer, and some PRs are urgent enough that they need to be merged without
an approval by another maintainer.
As a compromise between always needing a review and allowing maintainers to merge PRs they deem ready,
the `Triage` CI job will ensure that PRs cannot be merged until they've been open for 24 hours
(only counting hours that occur during the business week). After the triage period has expired, the
CI job will show up as "passed" and `BrewTestBot` will approve the PR, allowing it to be merged.
This gives all maintainers a reasonable opportunity to review every PR, but won't block any PR for lack
of reviews.
If the PR is urgent enough that it is necessary to bypass that 24 hour window, the `critical` label
can be applied to the PR. When this label is applied, the `Triage` CI job will immediately be
successful and `BrewTestBot` will approve the PR.
## CI
Every PR in `Homebrew/brew` runs a series of CI tests to try to prevent bugs from being introduced.
**A PR _must_ have passing CI before it can be merged**.
There are many checks that run on every PR. The following is a quick list of the various checks and what they represent:
- `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.
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
committed to the PR branch.
- `CI / test default formula (Linux)`: This runs `brew test-bot` 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 / 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
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.
Codecov should be used as a guide to indicate when more tests are probably needed, but it's unrealistic for
every line of code to have a test associated with it, especially when testing would require a slow
integration test. For this reason, it's okay to merge PRs that fail the Codecov check if necessary,
but this should be avoided if possible.
## Manpages and shell completions
Homebrew's manpages and shell completions are generated automatically by the `brew generate-man-completions` command.
Contributors are welcome to run this command and commit the changes in a PR, but they don't have to. If they don't,
a follow-up PR to make the necessary changes will be opened automatically by `BrewTestBot` once the original PR is
merged. These follow-up PRs can be merged immediately if the changes seem correct.
An update can be requested manually by triggering the workflow from the
[Update maintainers, manpage and completions](https://github.com/Homebrew/brew/actions/workflows/update-man-completions.yml)
section under the "Actions" tab. Click on the "Run workflow" dropdown and then the "Run workflow" button.
A PR will be opened shorty if there are any changes.

View File

@ -1,23 +1,129 @@
# Homebrew/homebrew-core Merge Checklist
# Core Maintainer Guide
## Quick merge checklist
A detailed checklist can be found [below](#detailed-merge-checklist). This is all that really matters:
- Ensure the name seems reasonable.
- Add aliases.
- Ensure it uses `keg_only :provided_by_macos` if it already comes with macOS.
- Ensure it is not a library that can be installed with
[gem](https://en.wikipedia.org/wiki/RubyGems),
[cpan](https://en.wikipedia.org/wiki/Cpan) or
[pip](https://pip.pypa.io/en/stable/).
- Ensure that any dependencies are accurate and minimal. We don't need to
support every possible optional feature for the software.
- When bottles aren't required or affected, use the GitHub squash & merge workflow for a single-formula PR or rebase & merge workflow for a multiple-formulae PR. See [below](#how-to-merge-without-bottles) for more details.
- Use `brew pr-publish` or `brew pr-pull` otherwise, which adds messages to auto-close pull requests and pull bottles built by the Brew Test Bot.
- Thank people for contributing.
Checking dependencies is important, because they will probably stick around
forever. Nobody really checks if they are necessary or not.
Depend on as little stuff as possible. Disable X11 functionality if possible.
For example, we build Wireshark, but not the heavy GUI.
For [some formulae](https://github.com/Homebrew/homebrew-core/search?q=%22homebrew%2Fmirror%22&unscoped_q=%22homebrew%2Fmirror%22),
we mirror the tarballs to our own BinTray automatically as part of the
bottle publish CI run.
Homebrew is about Unix software. Stuff that builds to an `.app` should
be in Homebrew Cask instead.
## Naming
The name is the strictest item, because avoiding a later name change is
desirable.
Choose a name thats the most common name for the project.
For example, we initially chose `objective-caml` but we should have chosen `ocaml`.
Choose what people say to each other when talking about the project.
Add other names as aliases as symlinks in `Aliases` in the tap root. Ensure the
name referenced on the homepage is one of these, as it may be different and have
underscores and hyphens and so on.
We now accept versioned formulae as long as they [meet the requirements](Versions.md).
## Merging, rebasing, cherry-picking
PRs modifying formulae that don't need bottles or making changes that don't
require new bottles to be pulled should use GitHub's squash & merge or rebase & merge workflows.
See the [table below](#how-to-merge-without-bottles) for more details.
Otherwise, you should use `brew pr-pull` (or `rebase`/`cherry-pick` contributions).
Dont `rebase` until you finally `push`. Once `master` is pushed, you cant
`rebase`: **youre a maintainer now!**
Cherry-picking changes the date of the commit, which kind of sucks.
Dont `merge` unclean branches. So if someone is still learning `git` and
their branch is filled with nonsensical merges, then `rebase` and squash
the commits. Our main branch history should be useful to other people,
not confusing.
Heres a flowchart for managing a PR which is ready to merge:
![Flowchart for managing pull requests](assets/img/docs/managing-pull-requests.drawio.svg)
Only one maintainer is necessary to approve and merge the addition of a new or updated formula which passes CI. However, if the formula addition or update proves controversial the maintainer who adds it will be expected to answer requests and fix problems that arise with it in future.
### How to merge without bottles
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 |
|---|---|---|
| **Commits look good** | rebase & merge _or_ squash & merge | rebase & merge |
| **Commits need work** | squash & merge | manually merge using the command line |
## Testing
We need to at least check that it builds. Use the [Brew Test Bot](Brew-Test-Bot.md) for this.
Verify the formula works if possible. If you cant tell (e.g. if its a
library) trust the original contributor, it worked for them, so chances are it
is fine. If you arent an expert in the tool in question, you cant really
gauge if the formula installed the program correctly. At some point an expert
will come along, cry blue murder that it doesnt work, and fix it. This is how
open source works. Ideally, request a `test do` block to test that
functionality is consistently available.
If the formula uses a repository, then the `url` parameter should have a
tag or revision. `url`s have versions and are stable (not yet
implemented!).
Don't merge any formula updates with failing `brew test`s. If a `test do` block
is failing it needs to be fixed. This may involve replacing more involved tests
with those that are more reliable. This is fine: false positives are better than
false negatives as we don't want to teach maintainers to merge red PRs. If the
test failure is believed to be due to a bug in `Homebrew/brew` or the CI system,
that bug must be fixed, or worked around in the formula to yield a passing test,
before the PR can be merged.
## Duplicates
We now accept stuff that comes with macOS as long as it uses `keg_only :provided_by_macos` to be keg-only by default.
## Removing formulae
Formulae that:
- work on at least 2/3 of our supported macOS versions in the default Homebrew prefix
- do not require patches rejected by upstream to work
- do not have known security vulnerabilities or CVEs for the version we package
- are shown to be still installed by users in our analytics with a `BuildError` rate of <25%
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.
## 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
[Acceptable Formulae](Acceptable-Formulae.md) requirements.
This is a guiding principle. 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.
This is a more practical checklist; it should be used after you get familiar with
[Maintainer Guidelines](Maintainer-Guidelines.md).
## Checklist
Check for:
- previously opened active PRs, as we would like to be fair to contributors who came first
- patches/`inreplace` that have been applied to upstream and can be removed
- comments in formula around `url`, as we do skip some versions (for example [vim](https://github.com/Homebrew/homebrew-core/blob/359dbb190bb3776c4d6a1f603a271dd8f186f077/Formula/vim.rb#L4) or [v8](https://github.com/Homebrew/homebrew-core/blob/359dbb190bb3776c4d6a1f603a271dd8f186f077/Formula/v8.rb#L4))

View File

@ -7,124 +7,38 @@ definitely not a beginners guide.
Maybe you were looking for the [Formula Cookbook](Formula-Cookbook.md)?
This document is current practice. If you wish to change or discuss any of the below: open a PR to suggest a change.
## Overview
All Homebrew maintainers are encouraged to contribute to all parts of the project,
but there are four main teams that maintainers tend to be a part of:
- `brew` maintainers: this team maintains the [`Homebrew/brew`](https://github.com/Homebrew/brew) repository.
See the [`brew` Maintainer Guide](Brew-Maintainer-Guide.md) for more details about being a `brew` maintainer.
- Core maintainers: this team maintains the [`Homebrew/homebrew-core`](https://github.com/Homebrew/homebrew-core)
repository. See the [Core Maintainer Guide](Core-Maintainer-Guide.md) for more details about being a core maintainer.
- Linux maintainers: this team maintains the [`Homebrew/linuxbrew-core`](https://github.com/Homebrew/linuxbrew-core)
repository. See the [Linux Maintainer Guide](Linux-Maintainer-Guide.md) for more details about being a Linux maintainer.
- Cask maintainers: this team maintains the [`Homebrew/homebrew-cask`](https://github.com/Homebrew/homebrew-cask),
[`Homebrew/homebrew-cask-drivers`](https://github.com/Homebrew/homebrew-cask-drivers),
[`Homebrew/homebrew-cask-fonts`](https://github.com/Homebrew/homebrew-cask-fonts) and
[`Homebrew/homebrew-cask-versions`](https://github.com/Homebrew/homebrew-cask-versions) repositories.
See the [Cask Maintainer Guide](Cask-Maintainer-Guide.md) for more details about being a cask maintainer.
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.
## 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.
## Quick checklist
This is all that really matters:
- Ensure the name seems reasonable.
- Add aliases.
- Ensure it uses `keg_only :provided_by_macos` if it already comes with macOS.
- Ensure it is not a library that can be installed with
[gem](https://en.wikipedia.org/wiki/RubyGems),
[cpan](https://en.wikipedia.org/wiki/Cpan) or
[pip](https://pip.pypa.io/en/stable/).
- Ensure that any dependencies are accurate and minimal. We don't need to
support every possible optional feature for the software.
- When bottles aren't required or affected, use the GitHub squash & merge workflow for a single-formula PR or rebase & merge workflow for a multiple-formulae PR. See [below](#how-to-merge-without-bottles) for more details.
- Use `brew pr-publish` or `brew pr-pull` otherwise, which adds messages to auto-close pull requests and pull bottles built by the Brew Test Bot.
- Thank people for contributing.
Checking dependencies is important, because they will probably stick around
forever. Nobody really checks if they are necessary or not.
Depend on as little stuff as possible. Disable X11 functionality if possible.
For example, we build Wireshark, but not the heavy GUI.
For [some formulae](https://github.com/Homebrew/homebrew-core/search?q=%22homebrew%2Fmirror%22&unscoped_q=%22homebrew%2Fmirror%22),
we mirror the tarballs to our own BinTray automatically as part of the
bottle publish CI run.
Homebrew is about Unix software. Stuff that builds to an `.app` should
be in Homebrew Cask instead.
### Naming
The name is the strictest item, because avoiding a later name change is
desirable.
Choose a name thats the most common name for the project.
For example, we initially chose `objective-caml` but we should have chosen `ocaml`.
Choose what people say to each other when talking about the project.
Add other names as aliases as symlinks in `Aliases` in the tap root. Ensure the
name referenced on the homepage is one of these, as it may be different and have
underscores and hyphens and so on.
We now accept versioned formulae as long as they [meet the requirements](Versions.md).
### Merging, rebasing, cherry-picking
Merging should be done in the `Homebrew/brew` repository to preserve history and GPG commit signing.
PRs modifying formulae that don't need bottles or making changes that don't
require new bottles to be pulled should use GitHub's squash & merge or rebase & merge workflows.
See the [table below](#how-to-merge-without-bottles) for more details.
Otherwise, you should use `brew pr-pull` (or `rebase`/`cherry-pick` contributions).
Dont `rebase` until you finally `push`. Once `master` is pushed, you cant
`rebase`: **youre a maintainer now!**
Cherry-picking changes the date of the commit, which kind of sucks.
Dont `merge` unclean branches. So if someone is still learning `git` and
their branch is filled with nonsensical merges, then `rebase` and squash
the commits. Our main branch history should be useful to other people,
not confusing.
Heres a flowchart for managing a PR which is ready to merge:
![Flowchart for managing pull requests](assets/img/docs/managing-pull-requests.drawio.svg)
#### How to merge without bottles
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 |
|---|---|---|
| **Commits look good** | rebase & merge _or_ squash & merge | rebase & merge |
| **Commits need work** | squash & merge | manually merge using the command line |
### Testing
We need to at least check that it builds. Use the [Brew Test Bot](Brew-Test-Bot.md) for this.
Verify the formula works if possible. If you cant tell (e.g. if its a
library) trust the original contributor, it worked for them, so chances are it
is fine. If you arent an expert in the tool in question, you cant really
gauge if the formula installed the program correctly. At some point an expert
will come along, cry blue murder that it doesnt work, and fix it. This is how
open source works. Ideally, request a `test do` block to test that
functionality is consistently available.
If the formula uses a repository, then the `url` parameter should have a
tag or revision. `url`s have versions and are stable (not yet
implemented!).
Don't merge any formula updates with failing `brew test`s. If a `test do` block
is failing it needs to be fixed. This may involve replacing more involved tests
with those that are more reliable. This is fine: false positives are better than
false negatives as we don't want to teach maintainers to merge red PRs. If the
test failure is believed to be due to a bug in Homebrew/brew or the CI system,
that bug must be fixed, or worked around in the formula to yield a passing test,
before the PR can be merged.
## Common “gotchas”
1. [Ensure you have set your username and email address properly](https://help.github.com/articles/setting-your-email-in-git/)
2. Sign off cherry-picks if you amended them (use `git -s`)
3. If the commit fixes a bug, use “Fixes \#104” syntax to close the bug report and link to the commit
### Duplicates
We now accept stuff that comes with macOS as long as it uses `keg_only :provided_by_macos` to be keg-only by default.
### Add comments
It may be enough to refer to an issue ticket, but make sure changes are clear so that
@ -143,21 +57,6 @@ is a good opportunity to do it) provided the line itself has some kind
of modification that is not whitespace in it. But be careful about
making changes to inline patches—make sure they still apply.
### Adding or updating formulae
Only one maintainer is necessary to approve and merge the addition of a new or updated formula which passes CI. However, if the formula addition or update proves controversial the maintainer who adds it will be expected to answer requests and fix problems that arise with it in future.
### Removing formulae
Formulae that:
- work on at least 2/3 of our supported macOS versions in the default Homebrew prefix
- do not require patches rejected by upstream to work
- do not have known security vulnerabilities or CVEs for the version we package
- are shown to be still installed by users in our analytics with a `BuildError` rate of <25%
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.
### Closing issues/PRs
Maintainers (including the lead maintainer) should not close issues or pull requests (note a merge is not considered a close in this case) opened by other maintainers unless they are stale (i.e. have seen no updates for 28 days) in which case they can be closed by any maintainer. Any maintainer is encouraged to reopen a closed issue when they wish to do additional work on the issue.
@ -178,7 +77,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 so can be self-merged once CI has completed.
## Communication

View File

@ -61,6 +61,7 @@
- [Maintainers: Avoiding Burnout](Maintainers-Avoiding-Burnout.md)
- [Maintainer Guidelines](Maintainer-Guidelines.md)
- [`brew` Maintainer Guide](Brew-Maintainer-Guide.md)
- [Core Maintainer Guide](Core-Maintainer-Guide.md)
- [Linux Maintainer Guide](Linux-Maintainer-Guide.md)
- [Cask Maintainer Guide](Cask-Maintainer-Guide.md)