Prev: Anyone know if Alvaro is OK?
Next: [GENERAL] trouble with to_char('L')
From: Mark Kirkwood on 27 Feb 2010 18:22
Greg Smith wrote:
>
>
> While I was in there I also added some more notes on my personal top
> patch submission peeve, patches whose purpose in life is to improve
> performance that don't come with associated easy to run test cases,
> including a sample of that test running on a system that shows the
> speedup clearly. If I were in charge I just would make it standard
> project policy to reject any performance patch without those
> characteristics immediately.
>

While I completely agree that the submitter should be required to supply
a test case and their results, so the rest of us can try to reproduce
said improvement - rejecting the patch out of hand is a bit harsh I feel
- Hey, they may just have forgotten to supply these things! The reviewer
can always ask, can they not? I would prefer to see the wiki say
something along the lines of "If you don't supply a test case you will
be asked for one before any further review can proceed..."

Cheers

Mark


--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Robert Haas on 27 Feb 2010 20:53
On Sat, Feb 27, 2010 at 6:22 PM, Mark Kirkwood
<mark.kirkwood(a)catalyst.net.nz> wrote:
> Greg Smith wrote:
>> While I was in there I also added some more notes on my personal top patch
>> submission peeve, patches whose purpose in life is to improve performance
>> that don't come with associated easy to run test cases, including a sample
>> of that test running on a system that shows the speedup clearly.  If I were
>> in charge I just would make it standard project policy to reject any
>> performance patch without those characteristics immediately.
>
> While I completely agree that the submitter should be required to supply a
> test case and their results, so the rest of us can try to reproduce said
> improvement - rejecting the patch out of hand is a bit harsh I feel - Hey,
> they may just have forgotten to supply these things! The reviewer can always
> ask, can they not? I would prefer to see the wiki say something along the
> lines of "If you don't supply a test case you will be asked for one before
> any further review can proceed..."

Agreed. Personally, I have no problem with giving a patch a brief
once-over even if it lacks an appropriate test case, but serious
review without a test case is really hard. That's one of the things
that slowed down rbtree a lot this last CommitFest. We should
probably try to make a point of trying to point this problem out to
patch submitters before the CommitFest even starts, so that they can
address it in advance.

....Robert

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

 | 
Pages: 1
Prev: Anyone know if Alvaro is OK?
Next: [GENERAL] trouble with to_char('L')