Re: [9.4 CF 1] The Commitfest Slacker List

<div dir="ltr"><br /><div class="gmail_extra"><br /><br /><div class="gmail_quote">On Tue, Jun 25, 2013 at 3:56 PM, Tom
Lane<span dir="ltr"><<a href="mailto:tgl@sss.pgh.pa.us" target="_blank">tgl@sss.pgh.pa.us</a>></span> wrote:<br
/><blockquoteclass="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"><divclass="im"><br /></div>FWIW, a large part of the reason for the commitfest
structureis that<br /> by reviewing patches, people can educate themselves about parts of the<br /> PG code that they
don'tknow already, and thus become better qualified<br /> to do more stuff later.  So I've got no problem with
less-experienced<br/> people doing reviews.<br /><br /> At the same time, it *is* fair to expect someone to phrase
theirreview<br /> as "I don't understand this, could you explain and/or improve the<br /> comments" rather than saying
somethingmore negative, if they aren't<br /> clear about what's going on.  Without some specific references it's
hard<br/> to be sure if the reviewer you mention was being unreasonable.<br /><br /> Anyway, the point I'm trying to
makeis that this is a community effort,<br /> and each of us can stand to improve our knowledge of what is
fundamentally<br/> a complex system.  Learn something, teach something, it's all good.<br /><br />                    
   regards, tom lane<br /><div class=""><div class="h5"><br /><br /></div></div></blockquote></div><br /></div><div
class="gmail_extra">Ijust wanted to give this the +1 but also want to add. As a novice back in the 8.4 cycle I wrote a
smallpatch implement boyer-moore-horspool text searches. I didn't have too much experience around the PostgreSQL source
code,so when it came to my review of another patch (which I think was even the rule back in 8.4 IIRC) I was quite clear
onwhat I could and could not review. The initial windowing function patch was in the queue at the time, so I picked
thatone and reviewed it along with Heikki. As a novice I did manage to help maintain a bit of concurrency with the
progressof the patch and the patch went through quite a few revisions from my review even before Heikki got a good look
atit.  I think the most important thing is maintaining that concurrency during the commitfest, if the author of the
patchis sitting idle waiting for a review the whole time then that leaves so much less time to get the patch into shape
beforethe commiter comes along. Even if a novice reviewer can only help a tiny bit, it might still make the difference
betweenthat patch making it to a suitable state in time or it getting bounced to the next commitfest or even the next
release.<br/><br />So for a person who is a little scared to put their name in the reviewer section of a patch, I'd
recommendbeing quite open and honest with what you can and can't review. For me back in 8.4 with the windowing function
patch,I managed point out a few places were the plans being created where not quite optimal and the author of the patch
quicklyput fixes in and sent updated patches, there was also a few things around the SQL spec that I found after
grabbinga copy of the spec and reading part of it. It may have been a small part of the overall review and work to get
thepatch commited but as Tom stated, I did learn quite a bit from that and I even managed to sent back a delta patch
which helped to get the patch more SQL spec compliant.<br /><br />I'm not sure if adding any a review breakdown list to
thecommitfest application would be of any use to allow breaking down of what the reviewer is actually reviewing.
Perhapspeople would be quicker to sign up to review the sections of patches around their area of expertise rather than
puttingtheir name against the whole thing, likely a commiter would have a better idea if such a thing was worth the
extraoverhead.<br /><br /></div><div class="gmail_extra">Regards<br /><br />David Rowley<br /></div><div
class="gmail_extra"><br/></div></div>