A Wednesday 14 May 2008, Ondrej Certik escrigué: > Hi, > > I read the recent flamebate about unittests, formal procedures for a > commit etc. and it was amusing. :) > I think Stefan is right about the unit tests. I also think that > Travis is right that there is no formal procedure that can assure > what we want. [snip]
For what is worth, Ivan and me were using patch peer review for more than a year in the PyTables project, and, although I was quite reluctant to adopt it at the begining, the reality is that it resulted in *much* better code quality to be added to the repository. Here it is the strategy ww used: 0. A ticket is opened explaining the feature to add or the thing to fix. 1. The ticket taker (i.e. the responsible of adding the new code) creates a new branch (properly labeled) for the desired modification/patch. The ticket is updated with a link to the new branch and the ownership of the ticket is transferred to the taker. 2. Once the modification is in the new branch, the ticket is updated explaining the actions done, and the ownership of the ticket transferred to the reviewer. 3. The peer reviews the code, and write suggestions in the same ticket about the new code. If the reviewer doesn't feel the need to suggest anything, he will write this in the ticket also. The ticket is transferred to the original author. 4. The original author should revise the suggestions of his peer, and if more actions are needed, he should address them. After this, he will transfer the ticket to the peer for a new review. 5. Phase 3 and 4 are repeated until an agreement is held by both parts, and the discussion remains in the ticket (one can temporarily tranfer part of the ticket discussion to the mailing list, if necessary). 6. After the agreement, the original author commits the patch in the temporary code branch to the affected branches (normally trunk and the stable branch of the project) and removes the temporary branch. The author has to tell explicitely in the ticket to which branches he has applied the new patch. 7. The ticket is closed. Of course, this works great with a pair programming paradigm (as was the case for PyTables), but for a project as NumPy there are more developers than just a pair, so you should decide how to choose the reviewer. One possibility is to form pairs by affinity, so that they can act normally together. Another possibility would be to force all the developers to subscribe to the ticket mailing list, and, for each ticket that requires a peer review, a call should be sent in order to gather a reviewer (who can offer as a volunteer by adding a note to the ticket, for example). I don't need to say that this procedure was not used for small or trivial changes (that were fixed directly), but only when the issue was important enough to deserve the attention of the mate. My two cents, -- Francesc Alted _______________________________________________ Numpy-discussion mailing list [email protected] http://projects.scipy.org/mailman/listinfo/numpy-discussion
