proposal for merge policy and maintainer roles #138

Open
moritz wants to merge 2 commits from MAINTENANCE into master
+72 -6
View File
@@ -3,6 +3,20 @@
All contributions should be made via a pull request. This is to ensure a
certain quality and consistency, that others can rely on.
## Roles
We differentiate between two roles:
- **Maintainers** take responsibility for the recipe. They are listed in the
[README.md](./README.md), are members of the
[traefik maintainers team](https://git.coopcloud.tech/org/coop-cloud/teams/traefik-maintainers)
and fulfil the responsibilities described below.
- **Contributors with merge access** Trusted people can open pull requests and merge them
moritz marked this conversation as resolved
Review

For transparency it would be nice yo also add mergers to the readme

For transparency it would be nice yo also add mergers to the readme
themselves in the cases described in [Pull Requests](#pull-requests).
They are listed in the [README.md](./README.md).
Being allowed to merge a pull request does not make somebody a maintainer.
## Maintainer Responsibilities
A recipe maintainer has the following responsibilities:
@@ -11,22 +25,74 @@ A recipe maintainer has the following responsibilities:
- Make image security updates within a week
- Make image major updates every three months
In order to fullfill these responsibilities a recipe maintainer:
In order to fulfill these responsibilities a recipe maintainer:
- Has to watch the repository (to get notifications)
- Needs to make sure renovate is configured properly
## Pull Requests
A pull request can be merged if it is approved by at least one maintainer. For
pull requests opened by a maintainer they need to be approved by another
maintainer. Even though it is okay to merge a pull request with one approval, it
is always better if all maintainers looked at the pull request and approved it.
As a rule, a pull request can be merged once it has been approved by at least
one maintainer other than its author. The exceptions below exist so that
security fixes and routine updates don't get stuck waiting for a review.
| Type of change | Approval needed |
| --- | --- |
| Security fix | No, can be merged immediately |
| Regular update | No, after a review window of about one week |
| Feature, extension or change of the default configuration | Yes, always |
Review

Can you elaborate on why you want to merge those changes directly? While looking through the Definition oft small changes they do not seem to be very urgent. I think I would prefer to not make a distinction between feature and small change

Can you elaborate on why you want to merge those changes directly? While looking through the Definition oft small changes they do not seem to be very urgent. I think I would prefer to not make a distinction between feature and small change
Review

Because often it's very annoying to wait for approval for such small changes.

  • a new abra.sh function: it doesn't break anything for other but for you it might be urgent to fix something. And if an abra.sh command also calls another abra.sh command it's important that it got released, else it could fail or lead o strange behaviour.
  • changes to the README.md: depends on what is changed. If you just fix some typos, an review is just a waste of time
  • Sometimes tests behave differently in the real test environment as local. Fixing a CI pipeline can be a process of try&error. And if you have automerge active these tests are relevant to get renovate to work again.
  • configuring renovate: same as above, renovate only reads it's configuration from main, therefore a merge is the only way to test configuration fixes.
Because often it's very annoying to wait for approval for such small changes. - a new `abra.sh` function: it doesn't break anything for other but for you it might be urgent to fix something. And if an abra.sh command also calls another abra.sh command it's important that it got released, else it could fail or lead o strange behaviour. - changes to the [README.md](./README.md): depends on what is changed. If you just fix some typos, an review is just a waste of time - Sometimes tests behave differently in the real test environment as local. Fixing a CI pipeline can be a process of try&error. And if you have automerge active these tests are relevant to get renovate to work again. - configuring renovate: same as above, renovate only reads it's configuration from main, therefore a merge is the only way to test configuration fixes.
| Small change that can't break the recipe | No |
### Security fixes
If a pull request contains a relevant security fix, the person who opened it is
allowed to merge it directly, without approval and without waiting.
The same applies to a pull request that fixes an update which contains a
security fix: if a version bump can't be merged as it is, because the new
version needs changes to the configuration, then the pull request that
makes that update work can be merged directly as well. Otherwise a broken
update would block a security fix.
Keep these pull requests as small as possible, so they contain nothing but what
is needed to get the fix out.
### Regular updates
If a pull request rolls out a regular update, there is a time window of about
one week for other maintainers to review it and to request changes. If no other
maintainer is available within this time window, the person who opened it is
allowed to merge it without approval.
### Features, extensions and default configuration
Any pull request that adds a feature or an extension, or changes the default
configuration, needs an approval, no matter who opened it. These are the
changes most likely to break existing deployments.
### Small changes
Small changes that definitely won't break the recipe can be merged without
approval, for example:
- a new environment variable that is commented out by default
- a new `abra.sh` function
- changes to the [README.md](./README.md)
- writing new tests
- configuring renovate
If a pull request falls into more than one of these categories, the stricter
rule applies. If you are unsure which category a pull request belongs to, ask
for an approval.
Even though it is okay to merge a pull request with a single approval, it is
always better if all maintainers looked at the pull request and approved it.
## Become a maintainer
Everyone can apply to be a recipe maintainer:
1. Watch the repository to always get updates
2. Simply add your self to the list in the [README.md](./README.md) and open a new pull request with the change.
3. Once the pull request gets merged you will be added to the [traefik maintainers team](https://git.coopcloud.tech/org/coop-cloud/teams/traefik-maintainers).
4. Join the room [#cc-|-traefik-maintenance:matrix.org](#cc-|-traefik-maintenance:matrix.org) and chat to other maintainers.
4. Join the room [#cc-|-traefik-maintenance:matrix.org](https://matrix.to/#/%23cc-%7C-traefik-maintenance:matrix.org) and chat to other maintainers.