An open team can be difficult to define. Being open is an ongoing process rather than a single event. It is a team-based way of thinking about your work. Let’s look at an example:
Subversion Terms Quick Guide
co – checkout or import
commit – check-in
editor – the place where you edit your comments
diff – show differences
file:/// – the path to the local server
full committer – someone with permission to change the Subversion product
svn – the Subversion command line
The Subversion team is a textbook case of a successful open team. They organize a large group of people and keep them involved with all aspects of their product. They strongly discourage code kingdoms and don’t allow any owner or author information in their source code.
Subversion is an open source team and all of their team communication is publicly available. Here is are portions of a real mailing list thread which took place between June 20, 2003 and July 30, 2003 on the Subversion developer mailing list. The thread started with a single email and the simple subject “svn commit performance.”
On June 20, 2003 Chia-liang Kao brought up an issue with the Subversion team. He had observed some performance problems with the commit command. This is the command which commits changes from a client machine to the Subversion server.
Like many performance problems, this one only happened under specific conditions. The first step Chia-liang took was to introduce the topic and give a decent amount of background information. The Subversion team tries to make raising an issue as simple as possible. In this case Chia-liang sent an email.
His email to the Subversion developer mailing list which said, “Recently I noticed the svn commit performs poorly before bringing up the editor.” He described where he was seeing the problem (during an import from the directory – svn co) and how to reproduce it. He also made a few guesses as to the cause of the problem:
Chia-liang Kao:
If i do a “svn co file:///repo/trunk svn.co”, edit a file in a deep directory, “svn diff” gives me the result immediately, while it takes 3 or 4 seconds for “svn commit” to bring up the editor. this is still acceptable.
But if i do “svn co file:///repo svn.co.full”, edit the same file in under trunk/’s deep direcotry. “svn diff trunk” still gives me the result immediately. but now “svn commit trunk” takes about 7 seconds to fire the editor. manually applying 6301:6302,6312:6313 for 0.24.2 takes 10 seconds even.
It seems the number of tags made the difference, since i didn’t notice this lag when i had no tags. but i think “svn commit trunk” should be irrevelent to tags/ ?
Chia-liang didn’t know the solution yet, or even if the problem was worth solving, he just saw a problem and brought it up with the team. He made sure to give as much information as possible so other people could follow what he was saying. He also wrote in a clear and easy-to-understand manner. The only problem I could find is that he didn’t use proper capitalization. Soon after he sent this email, he sent a second email with a theory of what could be causing the problem.
His first email did a good job representing the severity of the problem. Chia-liang doesn’t try to make this issue more important than it really is. He simply describes the behavior and how it will affect the user. He gives an objective assessment when he reports that the commit will take up to 10 seconds.
The first responder was C. Michael Pilato. Mike adds to the theory from Chia-liang’s second email. He also acknowledges Chia-liang’s question as one worth investigating by taking the time to understand his problem and respond to it. This is also implicitly validating Chia-liang’s work. By understanding and helping to resolve the issue Mike is showing that he thinks the issue was well-researched and is worthy of more of his time.
C. Michael Pilato:
And I’m gonna guess that what you saw was ‘lock’ files being written under tags/*, yes?
For reasons that I honestly don’t understand, ‘svn commit foo’ does a full recursive lock on foo’s parent directory. I’ve noticed this behavior for some time now, but haven’t had the chance to investigate.
By giving a reasoned and thoughtful response Mike has made it clear that he thinks this is a worthwhile topic for the team and encouraged Chia-liang to continue to investigate. Mike and Chia-liang discuss this issue throughout the day and by the end of the day Mike is working on a patch. A few more people enter the conversation and it turns out the problem is a little more difficult than the team originally thought. Mike and Chia-liang work on the patch for about three weeks with some help from other team members. On July 10, 2003 Chia-liang proposes a patch.
Chia-liang was a full committer and had been an active member of the project for a long time. He was a senior member of the team, but he didn’t just make the patch. His first step was to make the patch available for the team to review so he could get feedback. And the feedback that he got was more than just a quick code review.
Chia-liang received many comments on his patch. A particularly thorough comment came from Philip Martin.
Phillip Martin:
No, don’t use the probe functions, just use the information in the entries, call apr_hash_get to see if access batons exist, and call do_open to open any missing access batons.
Look at my original algorithm again:
Get P the path for adm_access
Get R the relative path of descendant and P
Split R into components.
For each component C in R
Get the access baton for P
If no access baton then error
Get the entry for C in P
If no entry for C then error
If C is not a directory return
Construct new P = P/C
Get access baton for P
If no access baton
Open new access baton for P
If that fails then error
Note the step “If C is not a directory return” this will allow the descendant path to be a file. At this point, before the algorithm returns, it should check that C is the last component of R, if not it should.
This comment is the epitome of good peer review. First, it is very specific. It avoids general comments and uses a laser-like focus to precisely address issues in the code. Second, he proposes a solution. It is easy to say, “this is wrong,” but it is much more helpful to give a suggestion for how to fix it. Third, this comment doesn’t have any blame or judgement. It simply presents the facts in a straightforward manner and lets them stand on their own. It doesn’t come across as a putdown or attack on Chia-liang’s abilities in any way. Finally, it is clearly written and easily understood. Even if you don’t know anything about the code in question you can still follow most of what Philip is saying. This superb example of team communication illustrates why the Subversion project has been so successful.
It’s not surprising that Chia-liang took these comments and made the suggested changes. Responding to a specific issue with large amounts of clear objective data almost always gets listened to. The team went back and forth a couple more times and then made the patch.
This patch succeeded for a number of reasons. Chia-liang Kao brought up the problem in an open-ended way. He did not try to force a solution; he just suggested one and then took feedback from the rest of his team. He also brought up the issue with the whole team rather than picking specific people. Not everyone responded, but it gave everyone a chance to be included if they wanted to be. When C. Michael Pilato made comments he stayed positive. He did a good job of expressing his concerns without becoming judgemental. When Philip Martin reviewed the patch he gave specific suggestions. He also used a straightforward nonjudgmental style. Addressing specific issues with constructive criticism is an implicit way of being supportive.
This issue took a little over a month to report, analyze, and fix. It involved six different people from the Subversion team and it was discussed at length. You might look at this example and wonder how the team ever gets anything finished if it takes six weeks and most of the core team to make a patch. In fact, the Subversion team has a long history of being an extraordinarily productive one. The explanation of this incongruity is one of the most unobvious aspects of peer review: helping other people will make you more productive.