On 2008-08-14 10:37, Brett Cannon wrote:
On Wed, Aug 13, 2008 at 11:54 PM, "Martin v. Löwis" martin@v.loewis.de wrote: [SNIP]
Anyway, if we're going to change policies around submitting code, I would much rather see peer review become a habit than adopt a tool like PQM.
The part where I'm skeptical about such a policy is that there might be a shortage of reviewers. What if a patch on Rietvield doesn't find a reviewer for a month or so? Many patches in the tracker sit there for years without any committer reviewing them.
It would be a change in culture for us, that's for sure. The question becomes whether the drop in patch throughput is justified by an increase of patch quality and stability in the code?
I don't know about other developers, but I tend to do code review by looking at the patch listings on the checkins list. While that doesn't allow finding problems before the checkin, it does allow finding them shortly afterwards and fairly quickly.
Of course, it's not as rigorous as using a special tool, but it also doesn't get in the way when the patches are in fact in good shape (which most of them are).
For everyday patches, I don't think we need a tool to help us with code review.
For larger chunks or new code that has to be reviewed on the tracker first, a peer review tool would make things easier, actually just a way to open a patch in the browser without having to first download it would already simplify things a lot ;-) (this currently doesn't always work due to MIME issues with the various different patch upload formats, e.g. .patch files, .txt files, etc.)
Discussing such larger chunks on the tracker before they go in is enough peer review and delay, IMHO.
In the end, some bugs will always get past all these barriers, regardless on how difficult you make it.