Prev: [HACKERS] 9.0 beta2 pg_upgrade command line parsing
Next: 9.0 beta2 pg_upgrade command line parsing
From: "Kevin Grittner" on 8 Jul 2010 11:36
Robert Haas <robertmhaas(a)gmail.com> wrote:

> Last year, the first CommitFest involved returning with feedback
> a lot of patches that had horribly bitrotted

That's an excellent point. If there is anyone out there who could
do an initial run to kick back patches with bitrot in the next week
or so, that would be a tremendous help.

> or had major design issues

That's a much harder problem. The set of people who can review for
that is rather smaller than the set who can see if a patch applies
without error.

> So giving people feedback now is really important to avoid having
> things pile up at the end of the release cycle.

Yeah, with only four review cycles, big items must get some review
early to avoid holding up the release at the other end.

> The good/bad news is that we don't have any major uncommitted
> patches floating around unmerged at this pont, as we did last
> cycle with HS and SR.

I'm not sure I understand what you mean by "unmerged" -- are you
excluding patches where they are in a git repository which is
merging in HEAD on a regular basis?

-Kevin

--
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 8 Jul 2010 12:29
On Thu, Jul 8, 2010 at 11:36 AM, Kevin Grittner
<Kevin.Grittner(a)wicourts.gov> wrote:
>> or had major design issues
>
> That's a much harder problem. �The set of people who can review for
> that is rather smaller than the set who can see if a patch applies
> without error.

Well, true. But reporting whether the patch applies without error is
about the most minimal review possible, so that's not saying much.
Many of the patches that need review are relatively small ones, so we
need people to check whether they work, see if they have docs and
regression tests, try to break them, and also - importantly - offer an
opinion on whether the proposed feature is actually useful. Of course
others may disagree, but you can't reach consensus if nobody's willing
to offer a starting point for discussion.

Reviewers who can actually analyze the code even a little bit can find
many more issues. Does it fit the style of the surrounding code? Are
there unnecessary or unrelated changes in the patch? And for extra
credit, does it have bugs? What I do frequently is find a
"comparable" - something similar in the existing code - and compare
them side-by-side. For each change, I ask myself whether it's there
because the new functionality is different from the old, or whether
it's arbitrary.

I agree that the larger features need a different kind of analysis,
and if those get left for more experienced reviewers I think that's
fine. Hopefully, some of those people will volunteer to help out; I
noticed that Itagaki Takahiro has already started reviewing, and Bernd
Helmle has signed up to review one of my patches. But there is no
shortage of things that can be looked at by people who are doing this
first the first time, especially if they have even a passing ability
to read C code.

>> The good/bad news is that we don't have any major uncommitted
>> patches floating around unmerged at this pont, as we did last
>> cycle with HS and SR.
>
> I'm not sure I understand what you mean by "unmerged" -- are you
> excluding patches where they are in a git repository which is
> merging in HEAD on a regular basis?

By unmerged I meant simply uncommitted. I know we have a number of
fairly big patches, but I don't think they're as complex as HS and SR,
and they are not leftovers that were too big to swallow during the 9.0
cycle.

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

--
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: "Kevin Grittner" on 8 Jul 2010 12:44
Robert Haas <robertmhaas(a)gmail.com> wrote:

> Kevin Grittner <Kevin.Grittner(a)wicourts.gov> wrote:
>>> or had major design issues
>>
>> That's a much harder problem. The set of people who can review
>> for that is rather smaller than the set who can see if a patch
>> applies without error.
>
> Well, true. But reporting whether the patch applies without error
> is about the most minimal review possible

I didn't mean to imply that only the two extremes of review ("does
the patch apply?" and "does this patch have a major overall design
flaw?") were the only things to try to address now; I was just
responding to your observation that these comprised a lot of the
activity of the first CommitFest last year, and that the latter is
harder to address without a review by a senior developer. I suspect
that one person could check for bitrot in all pending patches with a
one or two FTE day's effort, and if that's done within the next few
days, it might allow time for fixes before the start of the CF free
up more of the first week of the CF to more substantive review.

Nice comments later in the email, though; I hope you won't mind if
you find excerpts popping up in the code review Wiki pages. ;-)

-Kevin

--
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 8 Jul 2010 13:03
On Thu, Jul 8, 2010 at 12:44 PM, Kevin Grittner
<Kevin.Grittner(a)wicourts.gov> wrote:
> Nice comments later in the email, though; I hope you won't mind if
> you find excerpts popping up in the code review Wiki pages. �;-)

Not at all. Go for it...

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

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

First  |  Prev  | 
Pages: 1 2
Prev: [HACKERS] 9.0 beta2 pg_upgrade command line parsing
Next: 9.0 beta2 pg_upgrade command line parsing