ReviewGuide

Introduction

We have a script which automatically finds bugs with patches. Of course, it can't decide whether the patch is good or not, we need a human for this. The script does two things: Adds a patch tag and subscribes ubuntu-reviewers. The tag is used to track state as the patch goes through our process. The subscription is there to prompt us to start the process with a given bug.

As you complete each step of the process, you will mark your process by adding an appropriate 'patch-*' tag and removing the old one. Make sure to ALWAYS include a comment when you change the tag just like you would if you changed the bug status, so it's clear to later reviewers what happened. Do not remove the ubuntu-reviewers from the subscribers at any point.

Also, if you spot an abandoned patch that still looks relevant, that isn't tagged 'patch', feel free to add it manually and subscribe ubuntu-reviewers.

Workflow

  • Make a query for bugs with ubuntu-reviewers subscribed with only patch tag or without any reviewed tags for OperationCleansweep . Select one to work on.

  • Reproduce the bug step-by-step.
  • Review and test the patch.
    • If it does not work properly or needs more work, add the patch-needswork tag. Give the patch submitter some guidance on the rationale for the tag, and ask whether they are willing to update it to resolve outstanding issues. Consider adding the bitesize tag if it's a simple task, especially if the patch contributor hasn't reported back for some time.

    • If the patch works, subscribe yourself to the bug, forward the bug and patch upstream, and add patch-forwarded-upstream tag. If the change on the upstream package is debian specific, add the patch-forwarded-debian tag and forward to Debian.

  • Handle feedback from upstream.
    • If upstream accepts the patch, remove patch-forwarded-upstream tag, and add patch-accepted-upstream (or patch-accepted-debian) tag. Indicate the VCS revision and/or expected upstream/debian version/revision that will include the bugfix. If the change is significant enough to be fixed in Ubuntu, get the patch uploaded.

    • If upstream requests patch rework, add the patch-needswork tag, and indicate how to find the upstream feedback on the patch in the bug report. Ask the patch submitter whether they are willing to work with upstream to resolve outstanding issues.

    • If upstream rejects the patch, remove patch-forwarded-upstream tag, and add patch-rejected-upstream (or patch-rejected-debian) tag. Copy the reasoning for upstream rejection into the Ubuntu bug. Whether the changes get into Ubuntu in this case depends on the will of Ubuntu developers backing the change.

    • If upstream ignores the patch for a moderate amount of time, add the patch-forwarded-debian tag and forward to Debian (Handle feedback from Debian similar to handling upstream feedback )

  • If the patch is unnecessary or addresses something that does not need to be fixed, add tag patch-rejected, give reason in the comments, and if required close the bug to Won't Fix

Some packages are excluded from the list.

If the patch

  • is correct and fixes the problem
  • was accepted upstream or is important enough to bypass upstream's decision

get it into the sponsoring queue.

Working with Patches

If you haven't worked with patches extensively, you might want to head to our documentation about patches, because it will explain a lot of the most common questions like:

  • when to send patches to upstream and/or Debian
  • how to apply a patch
  • how to test a patch
  • and lots more.

If you have even more questions, check out our knowledge base.

Examples

  • 544242 This bug was opened with a patch provided by the reporter. It was subscribed by the subscription script with the patch tag. The patch was forwarded upstream, and recieved the patch-forwarded-upstream tag. After upstream accepted this patch, it recieved the patch-accepted-upstream tag and is ready to be fixed in Ubuntu.

  • 33288 The initial patch tag was changed to patch-needswork based on upstream comments.

  • 523349 The patch was forwarded to Debian and accepted there (patch-accepted-debian).

  • 544242 The patch was forwarded to Upstream GNOME (patch-forwaded-upstream) and after some discussion accepted (patch-accepted-upstream) there.

  • 462193 The patch was forwarded to Debian (because it just contained changes to the debian/ directory) and accepted there.

  • 582797 The patch was an Ubuntu-specific change, so a merge request was requested, following the sponsorship process, and was accepted.

ReviewersTeam/ReviewGuide (last edited 2011-06-28 11:25:19 by dholbach)