Re: MMAP Buffers

On Sat, Apr 16, 2011 at 8:48 AM, Greg Smith <greg@2ndquadrant.com> wrote:
> 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.

Give the OP a break - he was not "re-styling", he was clearly trying to make
crappily indented Postgres code readable in his editor.  Reading the patch
would not matter because the original code would still be crappily indented.

Yes, such patch is bad, but what should the proper response be in such
situation?

Hint: it can be both polite and short.

--
marko