Re: MMAP Buffers

Joshua Berkus wrote:
> Guys, can we *please* focus on the patch for now, rather than the formatting, which is fixable with sed?
>   

Never, and that's not true.  Heikki was being nice; I wouldn't have even 
slogged through it long enough to ask the questions he did before 
kicking it back as unusable.  A badly formatted patch makes it 
impossible to evaluate whether the changes from a submission are 
reasonable or not without the reviewer fixing it first.  And you can't 
automate correcting it, it takes a lot of tedious manual work.  Start 
doing a patch review every CommitFest cycle and you very quickly realize 
it's not an ignorable problem.  And lack of discipline in minimizing 
one's diff is always a sign of other code quality issues.

Potential contributors to PostgreSQL should know that a badly formatted 
patch faces an automatic rejection, because no reviewer can work with it 
easily.  This fact is not a mystery; in fact it's documented at 
http://wiki.postgresql.org/wiki/Submitting_a_Patch :  "The easiest way 
to get your patch rejected is to make lots of unrelated changes, like 
reformatting lines, correcting comments you felt were poorly worded etc. 
Each patch should have the minimum set of changes required to fulfil the 
single stated objective."  I think I'll go improve that text 
next--something like "Ways to get your patch rejected" should be its own 
section.

The problem here isn't whether someone used an IDE or not, it's that 
this proves they didn't read their own patch before submitting it.  
Reading one's own diff and reflecting on what you've changed is one of 
the extremely underappreciated practices of good open-source software 
development.  Minimizing the size of that diff is perhaps the most 
important thing someone can do in order to make their changes to a piece 
of software better.  Not saying something that leads in that direction 
would be a disservice to the submitter.

P.S. You know what else I feel should earn an automatic rejection 
without any reviewer even looking at the code?  Submitting a patch that 
claims to improve performance and not attaching the test case you used, 
along with detailed notes about before/after tests on your own 
hardware.  A hand wave "it's faster" is never good enough, and it's 
extremely wasteful of our limited reviewer resources to try and 
duplicate what the submitter claimed.  Going to add something about that 
to the submission guidelines too.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us