Project:Proxy Maintainers/Developer guide

From Gentoo Wiki
Jump to: navigation, search
Warning
This page is a work in progress by MGorny (talk | contribs). Treat its contents with caution.

This documents shortly documents policies and requirements for proxy-maint project members, i.e. developers merging the contributions from our users.

The three important rules

Let's define three important rules for doing proxy-maint work:

  1. Encourage users to maintain packages.
  2. Be reasonable yet firm while reviewing.
  3. Always test before merging.

Encouraging users to maintain packages

Please remember that the primary goal of proxy-maint project is to let users maintain packages. It is certainly not meant to serve as a dump for new packages without the incentive to support them afterwards, and it's not exactly meant to deal with one-time fixes to unmaintained (or maintained) packages. Therefore, if you see users that can potentially become maintainers, encourage them to do so.

For new packages, the case is rather obvious. If nobody is willing to maintain the newly submitted package, don't merge it. We're sorry but we can't be expected to take care of maintaining all the user-submitted ebuilds ourselves. Of course, nobody stops you from taking it over and maintaining it in your name. Just please don't do that on behalf of the project, and don't merge packages without a maintainer.

For existing packages, use your best assessment. If the package looks unmaintained or the contributor is putting a lot of effort into it, encourage him to become the maintainer. There is no harm in having multiple proxied maintainers for a package. Not only that improves the chance that it will actually be maintained but it also encourages people to work together.

Note that for packages outside proxy-maint that have an active maintainer, you should ask before adding proxied maintainers.

Being reasonable yet firm while reviewing

Reviews are at the core of proxy-maint work. They serve two important purposes: they make sure new ebuild enter in good quality, and they teach proxied maintainers how to write good ebuilds. While ideally we'd have both, you need to be able to tell what the user is interested in, and adjust.

Ideally, we'd want all packages to be maintained and top quality. However, that ideal is not always in reach. If the user submits a valid fix to an existing package, you may ask if he's interested in fixing some more problems with it. However, it would be unreasonable to reject the fix if he's not, or delay the fix by requesting a lot of additional immediate work.

For new packages, you should generally expect higher standards. After all, adding new packages with issues implies additional maintenance burden in the future. However, there are also limits on what to request. In some cases, it is more reasonable to suggest an easier workaround (say, restricting tests if they're very hard to fix) or ask the contributor to fix some remaining bugs later. It doesn't mean you should merge a new package if the contributor is actually refusing to fulfill reasonable requests.

You can also apply some simple fixes yourself, rather than asking the contributor to do them and resubmit. This saves both your and his time. However, you should tell him what you're doing so that he still learns from it.

Finally, being reasonable means making valid requests. Style changes are good if they improve long-term maintenance burden (this is why we encourage consistent variable ordering or dep sorting). However, requesting the user to follow your personal preferences is not always reasonable. If you have good arguments, explain it to the user. Make clear which of the requests are obligatory (policy), and which are optional (preference).

Testing before merging

Always test the packages before merging the changes. Nobody expects you to do thorough runtime testing or build all possible configurations of the package in question. However, you should do at least a minimal build/install test, with tests enabled. Do not merge obviously broken stuff.

If something looks suspicious, either verify it yourself or ask the user to check it. There's less harm in doing unnecessary test than in merging a bug you expected. If the tests fail, ask the user to investigate, and help him if you can. If tests aren't run but the package seems to have some, ask the user to try running them. If some tests fail and can't be fixed, it is usually a good idea to skip/xfail the specific tests rather than disabling tests entirely.

This is not really a matter of trust. Even if you process contributions from long-time proxied maintainers, test them. Even if things look simple, test them (real trivial changes may be limited to repoman/pkgcheck, use common sense). Testing can reveal issues that are specific to your configuration.

Important: do not leave users hanging

If you left review comments, make sure to answer any questions the user may have in reply, and update your review when new commits are pushed. If the user doesn't say that explicitly, a new CI report indicates that the pull request has been updated. If you can't find time or motivation to finish reviewing, please explicitly say so and discard your review.

Leaving user's update unanswered for a long time both creates negative feedback to prospective proxied maintainers, and often defers others from looking at the PR (apparently X is doing it, so let's wait for him to finish).

Detailed policies

Accepting new proxied maintainers

Please do not accept proxied maintainers who have not proved their skills. It's explained shortly in the User Guide. If someone submits a 'pure' request for taking a package over, you can merge it straight away only if you know that he has been doing good maintenance work in the past. If you don't know that, please ask him to solve some problem with the package (fix a bug, do an EAPI bump, anything).

If the package really doesn't have anything to do, you can try asking him to fix some other package or otherwise help you in development work. Anything that helps you evaluate that he will be able to maintain the package.

The main goal of this policy is to prevent reassigning packages just to remove the maintainer for inactivity shortly afterwards.

Reviewing

Please refer to the review suggestions of the GitHub project as a good base for working on pull request and other contributions (the reviewing rules are quite universal). However, a few notes need to be added to that.

On behalf of proxy-maint team, please focus on reviewing contributions to proxy-maintained (and potentially proxy-maintained) packages. Please do not merge fixes to other developers' packages on behalf of the project. If a proxied maintainer is interested in helping with some package, you can encourage him to become a co-maintainer. Of course, you can also merge those pull requests on your own behalf, with respect to the policies in place.

It is usually reasonable to assume that proxy-maint is not only about getting stuff merged but also about training prospective developers. Therefore, it is acceptable to expect higher standards than normally (and if the user asks, present this very reason to him). However, do not expect too much. After all, we expect proxied maintainers to stay here for a while, so sometimes it is reasonable to say: please fix this later.

Cross-proxied maintainer commits

Sometimes users submit pull requests to packages maintained by (other) proxied maintainers. Whenever that's the case, please encourage proxied maintainers to review those pull requests, and try to get their approval before merging. You can also suggest the user to become a co-maintainer if he seems interested.

Proxied maintainers proxied via developers

Some developers are handling proxied maintainers directly. We generally assume that proxy-maint has authority to commit on behalf of the proxied maintainer if it is listed in metadata.xml. If proxy-maint is not listed there, please do not merge without explicit permission of the proxy.

If the user is interested in working with proxy-maint team, feel free to ask him to add us to metadata.xml. Once that happens, proxy-maint can merge at will.

Maintainer inactivity

Proxied maintainers are expected to maintain their packages just like regular developers, including responding to bugs and feature requests. If the proxied maintainer seems inactive, please try to contact him. If the proxied maintainer is unable to maintain the package correctly, you can remove him and submit the package up for grabs appropriately.

Technicalities

Maintainer bugs

When merging the first contribution of a new proxied maintainer, you should create a new maintainer bug for him. However, it is easy to forget about it. Therefore, it is best to use the scripts to create all missing bugs when you remember.

user $cd proxy-maint-bugs
user $make

The script will scan your Gentoo repository checkout for all proxy-maintained packages and Bugzilla for maintainer bugs. It will create new maintainer bugs, as well as complain about those with outdated e-mail addresses and seemingly obsolete.

Note that you will need to obtain an API key from Bugzilla and place it in ~/.bugz_token.