On Fri, Apr 30, 2010 at 11:11 PM, Jeremy Dunck <jdu...@gmail.com> wrote: > As long as I'm here, how much interest is there in review board? > http://code.google.com/p/reviewboard/
I'm not sure. Like Alex, I've never used reviewboard itself. I have used codereview.appspot to review some patches (mostly because Alex and Mike Malone have uploaded patches there), but I haven't really had occasion to use it on a regular basis. My impression is that it's probably overkill for our current workflow. Broadly, patches fall into 5 categories: 1) The patch is completely correct 2) The patch is mostly correct, but has some minor problems (typos, simple edge cases) that are easily fixed 3) The patch is mostly correct, but has one or two questionable sections 4) The patch is solves the problem in the wrong way 5) The patch is missing some critical component like tests or docs In the case of a workflow that requires signoff from multiple parties before commit, smoothing the review process for cases 1-3 is vital. However, in Django's current workflow, cases 1 and 2 are absorbed by the committer doing their thing. Cases 5 is annoying, particularly if it looks like the patch would be in case 1 or 2 if only the contributor had written a test case. This is a major time sink, but a code review tool isn't needed here - the 'needs tests' flag is all that is required. Determining if a fix falls into Case 4 is a time sink, but once I've made the determination that a patch falls into case 4, a comment and the "patch needs improvement" flag works fine -- and in this case, works better than a line-based code review tool, since the comments are specific to the entire patch, not a specific line of code. So, if we were to integrate a code review tool, it would only be to support case 3. I don't have any concrete numbers to back me up, but my gut feeling is that case 3 isn't a time sink for me. Sometimes I do need to confirm why a patch works a particular way, and for those cases, I can drop a comment in Trac or on IRC without too much inconvenience. My intuition tells me that most of my commit time is spent determining if a patch fits into case 4, or adding tests for a patch in case 5. That said, if a code review tool was elegantly integrated into Trac (or any other bug tracker for that matter), it might be worthwhile. The vital keyword here is "integration". It needs to be as simple as "click on a patch in Trac, add a comment". If well integrated, it would be easier to leave a comment on a patch rather than on a ticket, so it becomes clear which patch is being reviewed. However, I don't know if this level of integration is even possible, and if it is possible, how difficult it is to achieve. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
