Open Week -- Patch Review Team -- Nigel Babu -- Tue, May 4
(01:01:26 PM) akgraner: nigelbabu, the floor is yours! (01:01:32 PM) nigelbabu: Hello folks! (01:01:43 PM) nigelbabu: I'm Nigel Babu from the Ubuntu Reviewers Team and I'm here to talk about patch review. (01:02:11 PM) nigelbabu: I've got a bit of ground to cover, so lets get started. (01:02:23 PM) nigelbabu: [SLIDE 1] (01:02:47 PM) nigelbabu: The slides for this talk is at http://people.ubuntu.com/~nigelbabu/patch-review.pdf (01:02:59 PM) nigelbabu: I'm not using them extensively, so if you don't have it, you're not missing much. (01:03:29 PM) nigelbabu: [SLIDE 2] (01:03:54 PM) nigelbabu: What does the Reviewers Team do? (01:04:08 PM) nigelbabu: Ubuntu has some amazing contributors contributing patches to bugs they encounter. (01:04:38 PM) nigelbabu: Over the past few years, we've not been able to handle this large influx of patches. (01:04:51 PM) nigelbabu: The Ubuntu Reviewers Team works to get these patches reported upstream and thus "give back" to the community. (01:05:37 PM) nigelbabu: The Ubuntu Reviewers Team get the patch, test it, and report it on the upstream bug tracker if the patch is working. (01:05:48 PM) nigelbabu: If a patch does not work as expected, the submitter is encouraged to submit a working patch to fix outstanding issues. (01:06:05 PM) nigelbabu: In case the submitter does not respond, the reviewer makes the modification giving credit to the submitter. (01:06:16 PM) nigelbabu: If a patch is critical enough, like a security issue or a major blocker, we work to getting it immediately into Ubuntu and forward upstream. (01:06:26 PM) nigelbabu: This may also qualify for an SRU to previous versions of Ubuntu. (01:07:18 PM) nigelbabu: [SLIDE 2] (01:07:54 PM) nigelbabu: How can you help? (01:08:02 PM) nigelbabu: While you can help in many ways, patch review is the important task for the Reviewers Team at the moment. (01:08:10 PM) nigelbabu: There are more than 1800 open bugs with patches attached that needs review. (01:08:23 PM) nigelbabu: I'll be covering how to do patch review today. (01:08:44 PM) nigelbabu: There are more ways that you can help, you can see https://wiki.ubuntu.com/ReviewersTeam/GettingInvolved for those. (01:09:19 PM) nigelbabu: [SLIDE 3] (01:09:47 PM) nigelbabu: [SLIDE 4] (01:10:11 PM) nigelbabu: Any quesetions so far? (01:10:39 PM) nigelbabu: moving on then (01:11:08 PM) nigelbabu: First step is to make a query for bugs with ubuntu-reviewers subscribed with only patch tag. Select one to work on. (01:11:40 PM) nigelbabu: you can see this query in https://wiki.ubuntu.com/ReviewersTeam/ReviewGuide, its a very big link (01:11:53 PM) nigelbabu: Next step is to reproduce the bug (01:12:20 PM) nigelbabu: Unless a reviewer reproduces the bug, its kind of difficult to check whether the patch works ;) (01:12:52 PM) nigelbabu: Next, review the patch and test it. (01:13:13 PM) nigelbabu: 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. (01:13:49 PM) nigelbabu: 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. (01:14:12 PM) nigelbabu: oh yes, subscribing yourself on any bug is very important, this allows you to keep track of the feedback (01:15:21 PM) nigelbabu: The upstream developers may give positive or negative feedback on the patch, the next step is to review those (01:15:44 PM) nigelbabu: If upstream accepts the patch, remove patch-forward-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. (01:16:54 PM) nigelbabu: An example of such a fix is a poppler bug that was fixed for lucid. Upstream accepted the patch a day after final freeze, but I talked to desktop team and got the fix for lucid. (01:17:21 PM) nigelbabu: 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. (01:17:36 PM) nigelbabu: pretty much straightforward step this one (01:17:55 PM) nigelbabu: If upstream rejects the patch, remove patch-forward-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. (01:18:17 PM) nigelbabu: Most of the times these changes may not make it into Ubuntu but some of them do make it ;) (01:18:39 PM) nigelbabu: 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 ) (01:19:03 PM) nigelbabu: upstream not responding is a situation that arises a lot of times, this step may alleviate the wait for some time. (01:19:23 PM) nigelbabu: And the final step is 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. (01:20:14 PM) nigelbabu: So, thats the entire review workflow, its a bit playing with tags, because LP needs a little bit more work into handling each patch. (01:20:25 PM) nigelbabu: Any questions on this? (01:22:03 PM) nigelbabu: [SLIDE 5] (01:22:19 PM) nigelbabu: I seemed to have messed up my slide numbers when I listed them earlier ;) (01:22:35 PM) nigelbabu: How to Join? (01:22:55 PM) nigelbabu: Does this sound exciting enough? Want to help us out? (01:23:13 PM) nigelbabu: To join the Reviewers Team you need to: (01:23:33 PM) nigelbabu: 1. Sign the Ubuntu Code of Conduct. (01:23:41 PM) nigelbabu: 2. Be part of Bug Squad. (01:23:47 PM) nigelbabu: 3. Have knowledge of Ubuntu/Debian packaging. (01:24:14 PM) nigelbabu: All the 3 are very critical, you need to know how to deal with upstream trackers and also generate debdiffs. (01:24:30 PM) nigelbabu: You don't need to be an expert though, we're all around and willing to help (01:24:41 PM) nigelbabu: All you need to do is ping one of the team members (01:25:09 PM) nigelbabu: The current team members are the folks who are members of https://launchpad.net/~ubuntu-reviewers (01:25:54 PM) nigelbabu: If you have installed the launchpad greasemonkey scripts, the reviewers show up with a small bandage icon (01:26:16 PM) ClassBot: A-R-R asked: I do want to help but I still need to learn Ubuntu/Debian packaging procedures. Where should I start learning from? Any recommended sources? (01:27:19 PM) nigelbabu: A-R-R: Excellent question, the best place is to look at the packaging guide in the MOTU documentation. (01:27:22 PM) nigelbabu: https://wiki.ubuntu.com/PackagingGuide/Complete (01:28:15 PM) nigelbabu: If you feel you're ready for being a team member, you can apply for membership in the launchpad team. (01:28:27 PM) nigelbabu: As with any Ubuntu development team, you are expected to already work as part of the team before you apply for membership. (01:28:53 PM) nigelbabu: Most of the work can be done without being a member, there is no blocker to do review if you are not a team member. (01:29:20 PM) nigelbabu: When you apply, be prepared to prove the review work that you've already done or have a current member vouch for your work. (01:29:30 PM) nigelbabu: Alternatively, come hang out on #ubuntu-reviews and do stuff until someone gets annoyed and bugs an admin to add you. ;) (01:29:50 PM) nigelbabu: Earlier in the yofel earned his membership that way :D (01:30:02 PM) nigelbabu: *Earlier in the day (01:30:46 PM) nigelbabu: Looks like I was kinda fast with the session, so I have plenty of time for questions :) (01:30:50 PM) nigelbabu: [SLIDE 5] (01:31:01 PM) nigelbabu: [SLIDE 6] (01:31:46 PM) nigelbabu: Patch Day (https://wiki.ubuntu.com/PatchDay) is now going on where we're mobilizing people to help review patches on May 5 in all time zones (adding to a total of 49 hours!). (01:32:30 PM) nigelbabu: We started at 1000 UTC today and we've covered close to 30 bugs by now! (01:33:03 PM) nigelbabu: The really big attraction of Patch Day is there will always be a review lead on the channel whom you can ask help and guidance. (01:33:14 PM) nigelbabu: The channel is #ubuntu-reviews btw (01:33:24 PM) nigelbabu: Feel free to participate and help out! (01:33:44 PM) nigelbabu: Patch Day runs till May 6 1000 UTC (01:33:57 PM) nigelbabu: So, any questions, please ask away (01:34:24 PM) nigelbabu: I have 30 minutes to take questions. I expected lots of them and kept time, don't disappoint me. (01:35:48 PM) nigelbabu: < vocx> About which kind of software are most patches? (01:36:05 PM) nigelbabu: Most of the time, its desktop applications that gets a lot of patches (01:36:39 PM) nigelbabu: The bugs that people find annoying and really want eliminated, we have one or two people annoyed enough to look at the code and find a fix :) (01:38:25 PM) ClassBot: A-R-R asked: What exactly is meant by 'upstream'? (01:39:39 PM) nigelbabu: Most of the time, Ubuntu doesn't develop all the applications. We use other applications to make a product called Ubuntu. (01:39:54 PM) nigelbabu: Upstream is the original developer of the applications we use. (01:40:33 PM) nigelbabu: For example, the gnome desktop environment has an upstream in the gnome project, that is where the original developers coordinate (01:41:58 PM) nigelbabu: Patch review is done of all patches attached to bugs in Launchpad, but reviewers need to work on all upstream trackers (01:42:16 PM) nigelbabu: It may the gnome bugzilla, the debian bts, the kde bugzilla, and many others (01:45:22 PM) nigelbabu: MononcQc> is the patch and review team usually composed of the same programmers who implement new features or applications? (01:45:35 PM) nigelbabu: There may be an overlap, but its not intentional. (01:46:15 PM) nigelbabu: We have a lot of very versatile contributors who're everywhere (like maco, I keep running into her) (01:46:29 PM) nigelbabu: So, you may see the same people in many teams :) (01:46:59 PM) nigelbabu: if you could prepond your questions with 'QUESTION: (01:47:16 PM) nigelbabu: that would be really great, it saves me the trouble of copy pasting (01:49:12 PM) maco: Just wanted to point out that patch reviewers don't have to be very good programmers (or in some cases, programmers at all), so there should be people who are reviewers but are not writing new features (01:51:09 PM) maco: <oppl> why would one submit a patch to launchpad and not submit a patch directly upstream? (01:51:34 PM) nigelbabu: If you can report it upstream, please do report it upstream. (01:52:08 PM) nigelbabu: Only, most people tend to submit(err.. not report) patches in Ubuntu bug tracker (launchpad) (01:52:39 PM) nigelbabu: Now, if the package is something that has someone caring abuot, things go fine (01:53:02 PM) nigelbabu: if its an orphan package, it ends up being on launchpad too long and we get a big backlog (01:53:44 PM) nigelbabu: and as maco reminds me, its easier to just report it in lunachpad and finding out the particular bug tracker and reporting it there. (01:54:15 PM) nigelbabu: report it in lunachpad *than* finding out the particular bug tracker and reporting it there (01:56:46 PM) nigelbabu: Also, dholbach will be taking 2 session on Friday about Ubuntu development and how to get involved, so that should help learn how to create debdiff, etc (02:00:09 PM) nigelbabu: so thats about it folks, thank you for participating :) (02:00:27 PM) akgraner: Thanks nigelbabu1 (02:00:34 PM) akgraner: Great session!