Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-09 Thread Michael Paquier 
On Wed, Mar 9, 2016 at 10:19 PM, Andres Freund  wrote:
> On 2016-03-10 06:14:25 +0900, Michael Paquier wrote:
>> IMO, during a review one needs to think of himself as a committer.
>> Once the reviewer switches the patch to "Ready for committer", it
>> means that the last version of the patch present would have been the
>> version that gained the right to be pushed.
>
> And one consideration there is whether you, as the committer, would be
> ok with maintaining this feature going forward.

Yes, that's a key point. Committers do the initial push and the
after-sales service as well.
-- 
Michael


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-09 Thread Andres Freund 
On 2016-03-10 06:14:25 +0900, Michael Paquier wrote:
> IMO, during a review one needs to think of himself as a committer.
> Once the reviewer switches the patch to "Ready for committer", it
> means that the last version of the patch present would have been the
> version that gained the right to be pushed.

And one consideration there is whether you, as the committer, would be
ok with maintaining this feature going forward.

But I think for less experienced reviewers that's hard to judge; and I
think asking everyone to do that raises the barriers to do reviews
considerably.  So I think we should somehow document that it's ok to
mark the patch as such, but that you're not forced to do that if you
don't want to.

Andres


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-09 Thread Michael Paquier 
On Wed, Mar 9, 2016 at 9:47 PM, Robert Haas  wrote:
> On Tue, Mar 8, 2016 at 9:44 PM, Craig Ringer  wrote:
>> On 9 March 2016 at 07:18, Tatsuo Ishii  wrote:
>>> Many of "needs review" state patches already have reviewer(s). Do you
>>> mean we want more reviewers in addition to them for such patches?
>>
>> Yeah. Personally I'm not too confident about what precisely is required to
>> move a patch from needs-review to ready-for-committer. I've done a chunk of
>> review for a number of patches, but I'm not always confident saying "all
>> clear, proceed".
>
> I think that if you've done your best to review it, and you don't see
> any remaining problems, you should mark it Ready for Committer.

IMO, during a review one needs to think of himself as a committer.
Once the reviewer switches the patch to "Ready for committer", it
means that the last version of the patch present would have been the
version that gained the right to be pushed.
-- 
Michael


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-09 Thread Robert Haas 
On Tue, Mar 8, 2016 at 9:44 PM, Craig Ringer  wrote:
> On 9 March 2016 at 07:18, Tatsuo Ishii  wrote:
>> Many of "needs review" state patches already have reviewer(s). Do you
>> mean we want more reviewers in addition to them for such patches?
>
> Yeah. Personally I'm not too confident about what precisely is required to
> move a patch from needs-review to ready-for-committer. I've done a chunk of
> review for a number of patches, but I'm not always confident saying "all
> clear, proceed".

I think that if you've done your best to review it, and you don't see
any remaining problems, you should mark it Ready for Committer.

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


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-09 Thread Robert Haas 
On Tue, Mar 8, 2016 at 6:18 PM, Tatsuo Ishii  wrote:
>> It's hard to miss the fact that there are an absolutely breathtaking
>> number of patches in this CommitFest - 80! - that are in the "needs
>> review" state.  We really, really, really need more review to happen -
>
> Many of "needs review" state patches already have reviewer(s). Do you
> mean we want more reviewers in addition to them for such patches?

Well, what we mostly need is more *reviews*.  Whether those are from
the reviewers already signed up or new reviewers is, IMHO, a secondary
consideration.

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


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Andres Freund 
On 2016-03-09 08:18:09 +0900, Tatsuo Ishii wrote:
> > It's hard to miss the fact that there are an absolutely breathtaking
> > number of patches in this CommitFest - 80! - that are in the "needs
> > review" state.  We really, really, really need more review to happen -
> 
> Many of "needs review" state patches already have reviewer(s). Do you
> mean we want more reviewers in addition to them for such patches?

Any review performed is worthwhile. Somebody having signed up to review
a patch shouldn't stop you. Especially if that signup was more than a
couple hours ago.

Andres


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Craig Ringer 
On 9 March 2016 at 07:18, Tatsuo Ishii  wrote:


>
> Many of "needs review" state patches already have reviewer(s). Do you
> mean we want more reviewers in addition to them for such patches?
>

Yeah. Personally I'm not too confident about what precisely is required to
move a patch from needs-review to ready-for-committer. I've done a chunk of
review for a number of patches, but I'm not always confident saying "all
clear, proceed".

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Tatsuo Ishii 
> It's hard to miss the fact that there are an absolutely breathtaking
> number of patches in this CommitFest - 80! - that are in the "needs
> review" state.  We really, really, really need more review to happen -

Many of "needs review" state patches already have reviewer(s). Do you
mean we want more reviewers in addition to them for such patches?

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Tom Lane 
Robert Haas  writes:
> Unique Joins - This patch has had a lot of review and discussion.  It
> would be best if Tom Lane looked at it.

Yeah, I'll pick it up soon.  I've basically been kicking as much as
I could down the road for the last couple of months, trying to get the
pathification changes done.  Now that that's in, I expect to be
substantially less AWOL from the commitfest.

> enhanced custom error in PLPythonu - No committer yet.  Tom Lane and
> Peter Eisentraut are the usual suspects for PL/python.  Again, I have
> neither the interest nor the knowledge.

I don't mind touching plpython at the C level, but this one requires
somebody who uses Python enough to have an informed opinion on the
tastefulness of the proposed language features.  That's not me.

regards, tom lane


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

Re: [HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Joe Conway 
On 03/08/2016 02:45 PM, Robert Haas wrote:
> OK, so I made a pass through the "Ready for Committer" patches in the
> current CF.  One I committed, several I replied to the thread with
> review comments and set back to "Waiting on Author". Here's where we
> are with the rest:

> plpgsql - possibility to get element or array type for referenced
> variable type - No committer yet.  I don't have enough interest or
> knowledge to want to handle this.

I'll try to move this one forward, although later in the week as I am
traveling all day tomorrow

> pl/pgSQL, get diagnostics and big data - No committer yet.  Seems like
> a reasonable concept.  A borderline bug fix.

possibly this one too...

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature

[HACKERS] fun with "Ready for Committer" patches

2016-03-08 Thread Robert Haas 
OK, so I made a pass through the "Ready for Committer" patches in the
current CF.  One I committed, several I replied to the thread with
review comments and set back to "Waiting on Author". Here's where we
are with the rest:

Silent data loss with ext4 / all current versions - It looks to me
like Andres is handling this.  I set him as the committer.
pg_resetxlog reference page reorganization - Peter Eisentraut wrote
this patch.  I assume he will commit it.  If not, I'm not sure why
anyone else should.
GCC 6 warning fixes - Ditto.
TAP test enhancements - It looks to me like Alvaro is handling this.
I set him as the committer.
Unique Joins - This patch has had a lot of review and discussion.  It
would be best if Tom Lane looked at it.  If not, one of us lesser
mortals will have to have a go.
index-only scans with partial indexes - Kevin has claimed this as committer.
Fix lock contention for HASHHDR.mutex - I guess I need to go revisit
this one, unless somebody else is willing to jump in.  I wouldn't mind
a few more opinions on this patch.
plpgsql - possibility to get element or array type for referenced
variable type - No committer yet.  I don't have enough interest or
knowledge to want to handle this.
pl/pgSQL, get diagnostics and big data - No committer yet.  Seems like
a reasonable concept.  A borderline bug fix.
enhanced custom error in PLPythonu - No committer yet.  Tom Lane and
Peter Eisentraut are the usual suspects for PL/python.  Again, I have
neither the interest nor the knowledge.

It's hard to miss the fact that there are an absolutely breathtaking
number of patches in this CommitFest - 80! - that are in the "needs
review" state.  We really, really, really need more review to happen -
and there's no place for that to come from except other people who
would like their own patches reviewed in turn, other than the limited
bandwidth of committers and of course the absolutely stunning efforts
of Michael Paquier to review everything in sight.  But that's not
going to be enough: we really, really need other people to be
reviewing also.

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


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