On 05/04/2012 12:24 AM, John Kinsella wrote:
Guys - during the developer on-ramp this week, we held a group discussion
around the topics of reviewing patch submissions, code maintainers, voting, and
overall care and management of the official source tree. We wanted to continue
the discussion on the developer list to look for the best way to set this up
for CloudStack and I volunteered to kick off the discussion.
For the overall scope of the discussion, I'm using "maintainer" to describe a
person who has been given commit rights to trunk, and who reviews potential code
modifications for inclusion into the CloudStack source. This is a separate responsibility
from that of setting direction and priorities for the CS project. Just want to be clear.
:)
I'm going to list out some of the points and goals we discussed, but this
probably isn't a complete list of things we should be considering:
Goals:
* Responsive maintainer team that nurtures involvement from the community by
providing friendly, useful, efficient responses to code modification submissions
* Responsibility vs authority - the goal is not to grant authority to
maintain the codebase, but to culture responsibility and respect for the
codebase and the community.
Points to consider:
* Maintainer scope: Is a maintainer responsible for certain parts of CS, or the overall
codebase? As CS moves to a more modular architecture, maybe a maintainer focuses on a
particular module? The concept of "super" maintainers was mentioned, a select
group of people who have the ability to effect change across the entire code base. That
one has such ability does not mean that it should be wielded often.
I like the notion of maintainers on a smaller level. However, there have
to be some people that maintain a reasonable overview of the complete
codebase to make sure things move somewhat reasonably in the same direction.
* Single or multiple maintainers: Be there a given scope for a module, sub-project, or whatever,
CS could adopt a model of a single "ultimate" maintainer who has to do all
reviews/commits, or a team of maintainers. The single person model provides "ultimate"
responsibility for that scope and a more predictable response to submissions, while a team
distributes the workload and minimizes the impact of a single person becoming
sick/distracted/trapped under bus/etc.
IMHO there should be multiple maintainers per sub-project/component/area
of interest. If there is only one there is bound to be conflict at some
point when a contributor and the maintainer cannot agree on some
decisions. Having a couple of extra people chime in in such situations
is always helpful.
* Code modification voting: General process for Apache projects is to vote
+1/0/-1. Question is, when do we use this? It probably isn't necessary for a 3
line commit.
One would hope that this should only be necessary for major changes, say
a group of people works in a fork for a while and does some major
re-architecting or whatever. When the branch comes back for merge it
appears reasonable to vote. For general fixes and changes voting appears
oevrkill to me.
* Responsiveness to patch submissions: One option to ensure prompt response to patch
submissions would be to have some form of guidance similar to "maintainers will
review and respond to patch submissions within 5 days" (pick a number). This isn't
really enforceable as the community is neither employed by nor reports to a single
entity, although repeated failures to meet the guideline could result in removal from the
maintainer position.
Another option to "motivate" maintainers to get to reviews could be to
automatically merge requests if they are X days past the "to be reviewed
within" time period. Maintainers having an interest that their
modules/components/areas of interest are not broken will not want
automatically checked in code that they had no chance to review.
* Patch testing: It would be great to have a test suite for a maintainer to
easily confirm that a patch does not inadvertently affect functionality/logic
outside the intended scope of the patch
Yes, some kind of test suite comprised on unit tests and functional test
would be great.
What have I left out? We referenced both the Linux (I can't find a good page
describing their process?) and Drupal
(http://drupal.org/contribute/core-maintainers) maintenance methods, but open
to any others we can look at. Please give any thoughts on the subject, and
definitely add any viewpoints/experiences/concepts that you may have.
In my experience people get discouraged quickly when proposed patches
get no action for an extended period of time (more than a week). I think
this has two reasons, contributors get the feeling that no one cares and
after about a week or so when someone comes back and asks a question
about the patch it is hard to come back and explain all the decisions
along the way. Therefore, prompt action on pull/review requests is
important.
In addition to the more fine grained points listed here it is also
important to set up some high level rules, such as "your patch breaks
the build, it gets reverted, no ifs, whens and buts" if it is considered
important to always have a non broken build, which IMHO it is.
My $0.02
Robert
--
Robert Schweikert MAY THE SOURCE BE WITH YOU
SUSE-IBM Software Integration Center LINUX
Tech Lead
rjsch...@suse.com
rschw...@ca.ibm.com
781-464-8147
