On Fri, May 4, 2012 at 9:52 AM, Robert Schweikert <rjsch...@suse.com> wrote: > 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.
+1. Here I like the typical hierarchy maintainer model. In CS probably 2 level is enough. > > >> * 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. IMO multiply maintainers may not be necessary. Because we're a community, rather than really dictatorship country. People would/should focus on the technically aspect, and if the contributor and maintainer can not reach an agreement, community can be involved in, as well as higher level maintainer. They're not necessary maintainer. Single maintainer mode doesn't necessary means SPOF. When the maintainer is on vacation, he/she or community can get a deputy maintainer as well. At least we don't have a Linus Torvalds here. :) Also, maintainer should be the expert on the one aspect, but it's not necessary means maintainer would override the community. > > >> * 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. Maybe a veto policy is more simple. "NACK", can be signed by the members of community(not only maintainers). Most of time, if there are no NACK, it should be fine. But if there are NACKs for the patch/feature, it would need more discussion, unless 1. reach an agreement, 2. or persuaded maintainer. Of course, "ACK" can also be signed, if necessary. ACK can be used by some expert(but not maintainers) in the fields. > > >> * 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. I don't think it's a good idea. If one maintainer didn't reply the patch for weeks, there would be something wrong with the maintainer. Contributor can bring everyone's attention on the mailing list, but automatically merge the patch? That's seems too far for me. > >> * 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. For certain part of code, maintainer can ask contributor to submit test along with the patch. I know there are some limitation with our unit test framework(e.g. test on some manager code), we need to work on that as well. > > >> >> 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. > And as a formal Linux kernel developer, I am very familiar with Linux maintenance mode. I may able to write something about it later. > > > 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. +1. -- regards Yang, Sheng > > 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
