Re: Formatting Curmudgeons WAS: MMAP Buffers

On Mon, Apr 18, 2011 at 12:52 PM, Joshua Berkus <josh@agliodbs.com> wrote:
> Robert, Tom,
>
>> Hm ... there are people out there who think *I* get high off rejecting
>> patches. I have a t-shirt to prove it. But I seem to be pretty
>> ineffective at it too, judging from these numbers.
>
> It's a question of how we reject patches, especially first-time patches.   We can reject them in a way which makes
thesubmitter more likely to fix them and/or work on something else, or we can reject them in a way which discourages
peoplefrom submitting to PostgreSQL at all. 
>
> For example, the emails to Radoslaw mentioned nothing about pg_ident, documented spacing requirements, accidental
inclusionof files he didn't mean to touch, etc.  Instead, a couple of people told him he should abandon his chosen
developmentIDE in favor of emacs or vim.  Radoslaw happens to be thick-skinned and persistent, but other first-time
submitterswould have given up at that point and run off to a more welcoming project. 

Actually, the first reply was a very polite reply from Heikki pointing
out the problem very gently and asking for a theory of operation.

Radoslaw replied and said that he understood the formatting problem,
but his editor was mangling it:

>> Yes, but, hmm... in Netbeans I had really long gaps (probably 8 spaces, from tabs), so deeper "ifs", comments at the
andof variables, went of out my screen. I really wanted to not format this, but sometimes I needed. 

That prompted one - ONE! - person to reply and suggest that the use of
another editor might work better.   At which point, we got an
apparently-exasperated note from you suggesting that a 10% performance
improvement wasn't enough (which I disagree with) and that it was
wrong for people to worry about whether they could read the patch well
enough to understand it (which I also disagree with).  Conceding that
some of the following discussion may have gotten a little harsh
(though frankly I think that was mostly directed at your remark, not
the OP), what prompted that original note?  Here it is:

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

So first of all, no it's not fixable with sed.  But secondly, writing
"*please*" here seems to evince a level of frustration which is
entirely out of proportion to the really rather mild comments which
preceded it.  What made you write it that way?

I think that the harshness of the reaction to your statement is a
reflection of some underlying frustration on my part and perhaps also
on the part of other reviewers - to this continual commentary that we
are not nice enough to people, especially newcomers.  Well, OK, maybe
we're not.  But you know what?  We're trying really hard, and getting
accused of being nasty when we actually weren't is kind of a tough
pill to swallow.  I would really like to see someone go back and look
at every patch from a newcomer that's been submitted in the last year,
and rate the reaction to that patch on an A-F scale.  Then let's have
a discussion about what percentage we did well on, and what percentage
we did poorly on, and how we could have done better.  When we actually
start raking someone over the coals, I think it's great and a helpful
service for you to jump in and say - hold on a minute, timeout.  But
in this case I think you were too quick off the trigger, and I don't
think that acting as if it's unreasonable to want a patch that
conforms to our submission guidelines is doing anyone any favors.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company