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.
- Click "register" from the top right.
- Enter in your details and register.
- Once you have your registration complete and can login, you should select "profile" and add in all the aliases you use to send patches to the list. This step helps patchwork assign patches to your account.
Lastly contact SiddheshPoyarekar or CarlosODonell to get added to the maintainers list on patchwork. This allows you to edit the state of any patch for the project. Please use this ability with caution; it is intended to allow maintainers to help with the maintenance of the patch list. We trust your judgement e.g. marking patches as committed if you see a commit for it, and marking patches under review that you are reviewing.
Following are the main points of the workflow:
- The subject line of the email that includes the patch must accurately reflects its content. The subject line is what appears automatically on patchwork and having an ambiguous or inaccurate subject line will only result in your patch being lost in the noise.
A reviewer may pick up a patch by delegating it to themselves by: following the link to the patch and changing the Delegate to value to their patchwork username. Do not change the status of the patch since it will then disappear from your default view. The patch would still be view-able in a list by modifying the Filters on the list.
- The following states may be used once the patch has reached a resolution or is being actively worked on by the submitter and one or more reviewers:
New - Nobody has reviewed it yet. This is the default status.
Under Review - Someone is looking at the patch. One may alternatively use the Delegate to field to indicate that they are interested in reviewing the patch.
RFC - Mark patches that aren't meant to be included as is and are posted as a proof of concept or just to solicit comments on the approach.
Accepted - There is consensus for committing this change.
Rejected - There is consensus that it's a bad idea.
Dropped - The patch was rejected and the developer dropped it.
Superseded - A newer version of the patch exists.
Deferred - Review of this patch is waiting another event to happen.
Change Requested - Reviewed and needs rework.
Committed - The change has been committed.
Not Applicable - The patch was cross posted and is not applicable to the project.
When the status of a patch is Change Requested, a submitter may post an updated version of the patch and mark the older version as Superseded.
- The submitter is still responsible for pinging the patch regularly if there have been no responses.
- Once review is done and the patch is pushed, the committer of the patch is responsible for changing the status of the patch.
Lifecycle of a patch
New patch submitted: New
A reviewer picks up the patch for review: Under Review (Note that the reviewer may also just use the Delegate to field so that the patch remains visible to other potential reviewers)
Reviewer thinks patch is OK: Accepted
Reviewer thinks patch is OK, but needs another opinion: New so that the patch becomes visible in the queue again
Reviewer thinks patch is OK, but another reviewer disagrees: Under Review
Reviewer thinks patch is not OK for glibc: Rejected
Submitter drops the patch: Dropped
Reviewer thinks patch needs changes: Change Requested
Patch committed: Committed
Submitter has posted an updated patch, then the old patch status: Superseded
Any maintainer thinks that the posted patch is an RFC and not an actual patch: RFC
Any maintainer sees a cross-posted patch not for glibc: Not Applicable
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.
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.