Patch Review Workflow

The glibc project now uses patchwork to track its patches; its instance is maintained at http://patchwork.sourceware.org.

Getting a Sourceware Patchwork Account

If you are one of the glibc MAINTAINERS then you need an account on the patchwork instance to maintain state for patches you review or post.

User Interfaces

We support 2 CLI interfaces and 1 web interface. The web interface is at http://patchwork.sourceware.org, and the patch queue can be seen directly here: https://patchwork.sourceware.org/project/glibc/list/. The two supported CLI interfaces are git-pw https://github.com/getpatchwork/git-pw and pwclient https://github.com/getpatchwork/pwclient.

Configuring git-pw

To configure git-pw you should setup token-based authentication for the REST API login. To do this you login to patchwork, select your name in the top right, select View Profile, and under Authentication generate the token. Open your ~/.gitconfig and add the authentication like this:

[pw]
        server = https://patchwork.sourceware.org/api/1.2
        token = XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
        project = glibc
        states = new,under-review,accepted,rejected,rfc,not-applicable,changes-requested,awaiting-upstream,superseded,deferred,committed,dropped

Where the XXX... is the token for authentication. The states are required because the project uses custom states. With this saved to ~/.gitconfig this will be the default project and states for git-pw unless a git local configuration overrides this. You may have multiple projects using patchwork, and so you can configure this on a per-repo basis.

Using git-pw

When you commit your patch you can automatically update the patch in patchwork like this:

git-pw patch update --commit-ref 4cab20fa49b3ea3e3454fdc4f13bf3828d8efd19 --state committed 38972

If you want to pull down a patch (whose ID you know) and review it you can do this:

git-pw patch apply 38972

You can work directly with a series and apply all patches in a series:

git-pw series apply 162

Reviewing a series

Patchwork allows you to review a series of patches, and you can quickly download and apply a series of patches like this:

git-pw series apply 359

Once you have the series downloaded you can use git's difftool feature to do higher level review like this:

git difftool -t meld -d HEAD~4..HEAD~3

If the series had 5 patches, and you diff HEAD~4..HEAD~3 you will be reviewing the 2nd of 5 patches. You can repeat the difftool invocation changing the number to review all patches in the series. This makes it easy to quickly review high-level changes at a glance with the local diff tool of your choice. In this example we use meld as the suggestion which is a good side-by-side difftool that gives full file context with colouring during the review. The use of -d allows you to see a full directory diff and see where files were deleted and created at a high level for review.

Patch Workflow

Following are the main points of the workflow:

Lifecycle of a patch

The state chart below should give a clear idea of the patch lifecycle. States marked in green are considered final states, i.e. patches in those states will not be visible in the default view.

glibc-patchwork-workflow-v2.svg

Patch Workflow Diagram

Notes to developers

When working on your own patches and using patchwork to maintain a list of actionable patches it is often useful to review rejected patches to determine which need more work using a different solution. The problem with this is that reviewing your rejected patches to see which need work gets harder and harder as the list gets longer and longer. There is a need to distinguish, for the developer only, the difference between rejected and dropped versus rejected and needs more work. If you are dropping the patch mark it with the Dropped state, otherwise keep it Rejected if you are going to do more work on the patch. Neither the Dropped or Rejected states are actionable and don't show up for reviewers looking to review patches.

None: Patch Review Workflow (last edited 2020-05-20 13:26:29 by CarlosODonell)