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(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us