On 16/12/14 13:37, Claudio Freire wrote: >> For the second project, I can skim through my inbox daily picking up >> specific areas I work on/are interested in, hit reply to add a couple of >> lines of inline comments to the patch and send feedback to the >> author/list in just a few minutes. > > Notice though that on the second project, the review is made "in the > air". You didn't build, nor ran tests, you're guessing how the code > performs rather than seeing it perform, so the review is "light" > compared to the first.
I think this doing developers in general a little injustice here. The general rule for a submitting patch to *any* project is: i) does it apply to the current source tree and ii) does it build and pass the regression tests? Maybe there is a greater incidence of this happening in PostgreSQL compared to other projects? But in any case the project has every right to refuse further review if these 2 simple criteria are not met. Also with a submission from git, you can near 100% guarantee that the author has actually built and run the code before submission. I have seen occasions where a patch has been submitted, and a committer will send a quick email along the lines of "The patch looks good, but I've just applied a patch that will conflict with this, so please rebase and resubmit". You mention about performance, but again I've seen from this list that good developers can generally pick up on a lot of potential regressions by eye, e.g. lookup times go from O(N) to O(N^2) without having to resort to building and testing the code. And by breaking into small chunks people can focus their time on reviewing the areas they are good at. Occasionally sometimes people do get a patch ready for commit but it fails build/test on one particular platform. In that case, the committer simply posts the build/test failure to the list and requests that the patchset be fixed ready for re-test. This is a very rare occurrence though. Also you mentioned about "light" review but I wouldn't call it that. I see it more as with each iteration the magnifying glass used to look at the code gets bigger and bigger. A lot of initial comments for the second project are along the lines of "this doesn't look right - won't this overflow on values >2G?" or "you're assuming here that A occurs before B, whereas if you run with -foo this likely won't work". And this is the area which I believe will have the greatest benefit for PostgreSQL - making it easier to catch and rework the obvious flaws in the patch in a timely manner before it gets to the committer. ATB, Mark. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
