Development
Name-and-shame to inspire patch reviews
Not many developers actually prefer reviewing patches; it is time-consuming and is often not as much fun as writing code. Nevertheless, it is an essential part of software development, so open source projects must find some way to motivate volunteers to undertake the task. PostgreSQL recently tried out a rather unusual approach: sending out a public list of all of those patch submitters who had not taken on a patch to review—which project policy mandates. The move ruffled a few feathers, but on the whole, more people seemed supportive of the idea than seemed offended by it.
At the heart of the issue is the project's practice of holding CommitFests—periodic pauses in PostgreSQL development during which the focus changes from new development work to patch review and merging. There have been many such CommitFests in recent years; four per calendar year. The current one started on June 16, targeting the upcoming 9.4 release. Josh Berkus, the current CommitFest Manager (CFM), emailed the pgsql-hackers list to announce it, and included a reminder about the project's policy that everyone who submits a patch is expected to pitch in and review at least one patch by someone else.
Berkus followed up on the subject on June 24, sending a list of the patch submitters who had not yet claimed a patch to review. The list was sizable, he said—at least as a proportion of the overall participants:
In addition, he noted, the patch-review policy (adopted for the
PostgreSQL 9.2 development cycle) indicates that not helping with
patch review would "affect the attention your patches
received in the future
". He further encouraged developers who
submit patches for their employer to let their supervisors know that
participating in the review process is considered part of contributing.
Naming names?
The tone of Berkus's email was certainly conciliatory, but there
were nevertheless a few people on the mailing list who seemed taken
aback by the act of publicly listing the names of the non-reviewers.
Dimitri Fontaine said " Several of these early critics referred to the process as "naming
and shaming," but Joshua Drake disagreed
with that characterization:
Drake quipped that participants should " Whether or not sending the names to the mailing list constitutes
"shaming" is, of course, a matter of opinion. Mark Kirkwood pointed out
that the subject line of Berkus's email ("[9.4 CF 1] The
Commitfest Slacker List") may have triggered that interpretation: " Furthermore, the only reason that it is important to remind list
participants of the CommitFest review policy (and perhaps the key reason to
have the policy in the first place) is that patch reviewers
are in short supply relative to submitters, and the project is trying
to rectify the imbalance.
As Kirkwood's email
pointed out, reviewing patches is a fundamentally more difficult task
than submitting them:
Now some might say "any review is better than no review"... I don't
think so - one of my patches a while was reviewed by someone who didn't
really know the context that well and made the whole process grind to a
standstill until a more experienced reviewer took over.
But, Tom Lane countered, a major
reason for the existing CommitFest process is that, by reviewing
patches, a submitter learns more about the code base. It is a community
effort, he said, " David Rowley agreed, adding that the CommitFest
offers a chance for reviewers to work side-by-side with patch
submitters, confident that the submitters will have the time to reply (and
perhaps iterate on the patch), since new development is on pause.
" Almost every free software project grapples with the fundamental
problem of cultivating new users and developers into experienced
contributors. PostgreSQL's patch-review policy for CommitFests is
clearly part of its program to do such developer cultivation. To that
end, it may not matter so much how the project notifies patch
submitters that they are also on the hook for a patch review, just as
long as the submitter participates. PostgreSQL gets better-reviewed
code, and the patch reviewer learns something in the process.
Seeing one's name of a list of people who still owe a review might be
surprising to some, but judging by the responses on the mailing list,
at least a handful of those listed got on the ball and signed up for a
patch to review. Consequently, it is hard to argue that the list of
names did not have a positive effect on the project. Furthermore, if
there were any hurt feelings, Berkus subsequently took steps to mend
them, asking the list for suggestions
on how to give public kudos to patch reviewers. What is still up in
the air is whether such kudos will take the form of public thanks,
physical gifts, or something else altogether.
I very much don't like
that idea of publishing a list of names
", and that he would
prefer sending out individual notices. Andreas Karlsson concurred, arguing that private
reminders are more effective. Berkus replied that private emails had been sent
a week earlier, but only a few of the recipients had picked up a patch
in the interim. And, as he noted
later, although the "submit one, review one" policy had been in place
for previous CommitFests, it had simply never been enforced in the past.
leave the ego at the
door
", but Fontaine replied that ego was not the
issue at all. Emailing the list of non-reviewers to everyone, he said,
would discourage contributors from participating. Instead, he said, the
project ought to do something positive to reward the people who
do review patches. Maciej Gajewski did not object to publicizing the list of
names, but argued that the policy
itself needed to be more prominently communicated, so that "
newbies like myself (who wouldn't even dare reviewing patches
submitted be seasoned hackers) are not surprised by seeing own name on
a shame wall
".
Let's review
Lol - Josh's choice of title has made it a small shame wall (maybe
only knee high).
" Nevertheless, the content of Berkus's email
simply reminded recipients of the review policy in pretty straightforward
language. One might argue that working in the open dictates public
scrutiny of all sorts of potentially embarrassing incidents; for
instance, is it "shaming" to have a patch rejected?
and each of us can stand to improve our
knowledge of what is fundamentally a complex system
".
Less-experienced reviewers are no problem, he said, and it is fair to
expect reviewers to ask questions or request better comments and
explanations if they find something unclear.
Even if a novice reviewer can only help a tiny bit, it might
still make the difference between that patch making it to a suitable
state in time or it getting bounced to the next commitfest or even the
next release.
" Rowley said that he had started out as a novice
in the PostgreSQL 8.4 cycle, and learned a great deal by reviewing
patches and by being perfectly clear about what he did and did not
understand. Other new reviewers, he said, would do well to be up
front about the limits of their experience as well.
Brief items
Quotes of the week
Introducing Daala
Daala is a new video codec from Xiph.org; the project has posted a detailed explanation of how it works. "Daala tries for a larger leap forward— by first leaping sideways— to a new codec design and numerous novel coding techniques. In addition to the technical freedom of starting fresh, this new design consciously avoids most of the patent thicket surrounding mainstream block-DCT-based codecs. At its very core, for example, Daala is based on lapped transforms, not the traditional DCT."
PHP 5.5.0 released
Version 5.5.0 of the PHP language is out. New features include generator and coroutine support, a new finally keyword, a better password hashing API, a new opcode caching mechanism, and more.Changes coming for systemd and control groups
Lennart Poettering posted a message to the
systemd-devel mailing list that outlines some changes coming. It refers to
an earlier message that started describing
differences coming in the kernel's control group (cgroup) implementation
and their effects on systemd. Those cgroup changes were discussed in February 2012 and
cgroup maintainer Tejun Heo decided to
eventually drop
multiple cgroup hierarchies in March 2012. The basic idea is that the
single hierarchy will also have a single manager on the system and all
changes to the cgroups will need to go through that manager. For systems
that use it, systemd will become the manager of cgroups, so other tools
that want to make changes will no longer follow the PaxControlGroups
recommendations and will instead make D-Bus requests to systemd.
Non-systemd systems will need to come up with their own manager and
management interface once the cgroup changes are made in the kernel (for
which the timeline is unclear). "In the long run there's only going to be a single kernel cgroup
hierarchy, the per-controller hierarchies will go away. The single
hierarchy will allow controllers to be individually enabled for each
cgroup. The net effect is that the hierarchies the controllers see are
not orthogonal anymore, they are always subtrees of the full single
hierarchy.
"
osm-gps-map 1.0.0 released
Version 1.0.0 of the osm-gps-map library has been released. The library is a general-purpose mapping widget supporting OpenStreetMap and other online services, as well as GPS tracks. This is the first stable release in more than a year, and adds support for GTK+ 3.0.
Firefox 22 is now available
Mozilla has released Firefox 22. Among the changes in this version are WebRTC support being enabled by default, major performance improvements to ASM.js, playback-rate adjustment for HTML5 audio and video, the addition of support for HTML5 <data> and <time> elements, social networking management and, best of all, a built-in font inspector.
LibreOffice 4.0.4 available
The Document Foundation has announced LibreOffice 4.0.4, which is expected to be the final minor release before the announcement of LibreOffice 4.1.0, currently slated for late July. A number of improvements to interoperability with proprietary office suites land in 4.0.4, in addition to the usual host of bug- and regression-fixes.
Newsletters and articles
Development newsletters from the past week
- Caml Weekly News (June 25)
- What's cooking in git.git (June 20)
- What's cooking in git.git (June 25)
- OpenStack Community Weekly Newsletter (June 21)
- Perl Weekly (June 24)
- PostgreSQL Weekly News (June 23)
- Ruby Weekly (June 20)
Sanders: A picture is worth a thousand words
On his blog, Vincent Sanders peers in on the process of rendering an image in a web browser. While conceptually simple, there is actually a fair amount of work that has to be done to get from the bits over the wire to the image on the screen. "In addition the image may well require tiling (for background gradients) and quite complex transforms (including rotation) thanks to CSS 3. Add in that javascript can alter the css style and hence the transform and you can imagine quite how complex the renderers can become."
Desperate times call for Tux Paint (opensource.com)
Opensource.com has an interview with Tux Paint creator Bill Kendrick. Tux Paint is a drawing program targeted at children, but is used by adults, particularly teachers. "I didn't design Tux Paint to be a tool that's only used for teaching art, I designed it to be a tool like a piece of paper and pencil. Some schools use it to create narrated storybook videos. Other schools use it for counting and math. Ironically, these days, after being exposed to things like Mr. Pencil Saves Doodleburg on my son's Leapster Explorer (which, by the way, runs Linux!), I'm beginning to think Tux Paint needs some additional features to help teach concepts about art and color. Or at least a nicely done set of curricula."
California Says the Bitcoin Foundation Is a Money-Transferrer (Wired)
Wired notes a potentially major complication for the Bitcoin Foundation: the California Department of Financial Institutions has decided that the foundation is in "
the business of money transmission
", which would subject it to regulatory scrutiny, and has sent it a cease-and-desist letter. For its part, the Bitcoin Foundation does not seem to feel that the letter is a source of trouble, but evidently other US states are similarly taking a close look at how Bitcoin operates.
Page editor: Nathan Willis
Next page:
Announcements>>