Project:Codeberg/Pull requests

From Gentoo Wiki
Jump to:navigation Jump to:search

Creating pull requests

Important
Please do not fork the repository on Codeberg. The recommended AGit workflow does not require that, and it is much more space efficient.

Please use AGit to submit pull requests. First, add a Codeberg remote:

user $git remote add codeberg [email protected]:gentoo/gentoo.git

Then, to create a new pull request:

user $git fetch codeberg
user $git push codeberg HEAD:refs/for/master -o topic="$TOPIC"

The TOPIC is used to identify the pull request, use the same topic when pushing updates to it. It is also used to determine the initial pull request summary, it can be changed later via the website.

When force-pushing, you need to use the force-push parameter, i.e.:

user $git fetch codeberg
user $git push codeberg HEAD:refs/for/master -o topic="$TOPIC" -o force-push=true

Pull request assignment

The new pull requests are automatically assigned using a cronjob run every 4 minutes. The assignment script leaves a comment @-mentioning appropriate developers/teams and setting labels appropriately.

The assignment is performed based on the list of changed files. The script reads the current maintainers from the metadata.xml files, and maps them to Codeberg developers/teams. It should be noted that the files from the current ::gentoo version are used rather than ones from the pull request.

The following additional rules apply:

  • New and unmaintained packages are assigned to Project:Proxy Maintainers as candidates for proxy maintenance.
  • Eclass and profile changes are not assigned automatically, and instead assumed to be handled by other assignees of the pull request (if any).
  • Pull requests involving packages with more than 5 unique sets of maintainers are not assigned automatically (to prevent spam).
  • Packages whose maintainers have no Codeberg account (as in, none of the maintainers do have one) are marked as non-assignable.

If the script was able to find at least one assignee for each of the modified packages, the pull request is marked assigned. Otherwise it is marked need assignment.

In order to prevent automatic assignment, please set the assigned label or add at least one explicit assignee. In order to have the pull request re-assigned, remove all assignees (if any) and the appropriate label. The script will automatically remove the previous assignment comment.

Maintainer address verification

The assignment script additionally verifies that the e-mail addresses of the maintainers are registered on the Gentoo Bugzilla. If an unregistered address is found, the assignment comment contains a verbose warning about it.

When the submitter fixes the issue, or the package list changes, you can remove the assigned label in order to force reassignment. This will also repeat the verification.

Pull request CI

All pull requests are subject to CI runs that run pkgcheck on the whole repository. The CI script should take around 15–20 minutes for a single pull request, and on every run it sets check status to indicate the position in queue.

The CI run consists of pregenerating caches and running pkgcheck on the whole repository. This makes it possible to catch indirect dependency breakages resulting from the changes. If any issues are found, the result is compared to the repository state on which the PR was last rebased. The report indicates appropriately which issues are new to the pull request, and which are persisting (i.e. could be fixed by rebasing).

In order to rerun the CI, force push new commits over the pull requests. The script will detect changed commit identifier, and rerun the checks. it is currently not possible to disable CI on a pull request.

Warning
The CI checks only report severe problems. Furthermore, pkgcheck is imperfect (yet very fast) and may fail to notice some subtle breakages (especially if USE flag masking/forcing) is involved. CI is not an excuse not to run pkgcheck scan, at least for the packages being committed.

Pull request reviews

Who is eligible to review?

Everyone can review pull requests. However, please make sure that your comments are correct and on topic. It is a bad experience for our users if they are confused by incorrect suggestions, and then asked to revert them.

Pull request reviews are non-binding. They are supposed to improve the pull request and make it more likely to be accepted as-is by the assignee. The assignee may choose not to accept a pull request independent of positive or negative reviews.

How to review?

A different levels of review are appropriate to the different kinds of commits. In particular:

  • If a pull request affects a small part of ebuild (adding a patch, fixing a small bug), don't request the author to fix irrelevant bugs (unless he explicitly wants to).
  • If a pull request does a version bump, don't ask the author to fix minor bugs that are unchanged since the previous version (unless he explicitly wants to).
  • If a pull request does an EAPI bump, it might be reasonable to request changes semi-relevant to the EAPI bump.
  • If a pull request submits a new ebuild, it is highly desirable to bring it to top-quality before being committed.
  • If a pull request requests proxied maintenance of an existing ebuild, it is desirable to ask the new maintainer to fix some bugs, however not necessarily all of them at once.

Be clear in your comments. Avoid unnecessary nitpicking, and even more importantly — avoid wars between developers over ebuild style. If something is purely your preference or recommendation, state that clearly and explain why. If you're not sure, say that, suggest how to check. Feel free to @-mention other developers or teams (QA for technical issues, licenses for license doubts, python for Python doubts, etc.) to get help.

For simple additional changes (revbump, typo fix), don't request the submitter to update the pull request. Instead, leave a comment stating what is wrong and that it will be fixed by the person merging it, so that the submitters knows how to avoid the problem in the future. Add a 'revbump on merge' or 'fix on merge' label appropriately, so that people won't merge it accidentally.

How to submit reviews?

Preferably, use the Codeberg review feature to submit reviews. This makes it possible to combine all comments into a single mail rather than sending one mail for each comment.

You can either review the final result of the pull request (Files changed tab) or single commits separately. In order to add a comment, click a line to be commented on, write the comment and use Start review. This will cause this and your further review comments to be marked pending — which also allows you to modify or remove them before sending.

Please note that Codeberg stores your pending, so you can freely navigate through multiple commits and tabs, or even leave the pull request without losing your input. Once ready to submit, navigate to the Files changed tab and use the Finish review button on top.

You can write the general comments here (e.g. applying to the whole pull request, if necessary) and additionally decide if you approve or request changes to the pull request. Neither is obligatory — you can also use the review feature to leave a few neutral comments in a single mail.

Merging pull requests

Pull request approval

Please remember that pull requests follow the same rules as other commits to the Gentoo repository. Therefore, you can only merge a pull request if you would be permitted to commit the changes to the Gentoo repository. Furthermore, you take responsibility for the changes you are merging.

In particular, changes affecting packages maintained by other developers/projects may require their permission to be merged. If the maintainers do not use Codeberg, you may need to use other channels to contact them and obtain approval for the merge.

Common tips

Adding the git remote

Most of the methods listed here require you to fetch pull requests from Codeberg. In order to do that most conveniently, create a new remote for Codeberg and enable fetching of pull request refs:

user $git remote add codeberg https://codeberg.org/gentoo/gentoo.git
user $git config --add remote.codeberg.fetch '+refs/pull/*/head:refs/remotes/codeberg/pull/*'

Afterward, each call to:

user $git fetch codeberg

will fetch new pull requests and expose them as codeberg/pull/NNNN refs.

Closing pull requests

There are two ways of closing a pull request:

  1. Via a Merges tag.
  2. Manually.

One must add a Merges tag to your commit message to make Codeberg close the pull request properly:

CODE Example git commit message
dev-foo/bar: fix fnord

Bug: https://bugs.gentoo.org/mmmmmm
Merges: https://codeberg.org/gentoo/gentoo/pull/NNNN
Signed-off-by: A Contributor <[email protected]>
Signed-off-by: A Developer <[email protected]>

Inline merges

Using git am

It is possible to use Codeberg generated patches to avoid adding the Codeberg repository locally. It should be noted, however, that git am is weaker than git cherry-pick at conflict resolution.

pram

app-portage/pram is the recommended tool for grabbing patches from Codeberg pull requests:

Using pram is preferable as it checks for Signed-off-by and also adds the necessary Part-of and Merges git trailers to commits.

Manually
user $wget -O - https://codeberg.org/gentoo/gentoo/pulls/NNNN.patch | git am --signoff

Using git cherry-pick

It is possible to cherry-pick instead via the pull done earlier. In order to cherry-pick all new commits from the pull request, do:

user $git cherry-pick master..codeberg/pull/NNNN

Afterwards, edit the commits as necessary, rebase and sign them and push. Make sure to manually add the Part-of and Merges trailers if doing this.