Re: [HACKERS] Patch Submission Guidelines

Many patch submitters discover that they fall foul of various "you
should have done"s at a late stage of the patch review process.
These include the usual:
- major feature change not discussed on -hackers or elsewhere first
- patch in wrong format
- performance patch, yet no performance test results to prove benefit
- no accompanying doc patch
- won't work on various ports (and it needs to)
etc..

In contrast, the documentation and translation process is extremely well
documented; this may be by design.

I would like to suggest that we increase substantially the FAQ entries
relating to patch submission. By we, I actually mean please could the
committers sit down and agree some clarified written guidelines?

There is nothing wrong right now with the level of quality of patches
that get accepted, so I do not wish to discuss lowering or increasing
the quality bar. What I do want to discuss is how to increase the
efficiency of the patch submission process so that senior committers
spend less of their time (our most critical resource) on poor quality
submissions (however that is judged) and also that patch submitters also
have fast feedback on missing requirements.

A clear FAQ entry or checklist can be applied easily by more casual
readers of the -patches list, allowing errors to be pointed out quickly
by non-committers and any missing requirements rectified. Written
guidelines are also much more easily translated than no guidelines at
all, benefiting non-native English speakers considerably.

Some of the above guidelines are clearly explained in FAQ, others not. I
would also want to add to the Developer page of the website something
along the lines of "Interested in developing for PostgreSQL? Please read
the <A>patch submission guidelines</A> before you begin work since only
the highest quality patches will be accepted."

I believe if we do this we will have more patches produced, reviewed and
committed from our available resources, as well as more hackers more
regularly willing to face the challenges of getting a quality patch
accepted. In the end we will live and die by the number of people
submitting and how many of those go on to become regular contributors
(should I say "serial hackers"?)

Bruce currently maintains much of this material, so I want it to be
known that this is specifically not a criticism of his work. This is
just an earnest attempt to increase the efficiency of the current
process, so patch authors can move quickly onto their next patch.

[Increasing the quality of my own submissions is a necessary act in this
process, though I hope these thoughts can be considered outside of my
own involvement and experience.]

It's probably also time for the annual discussion about when is the next
patch submission deadline. ;-)

Best Regards, Simon Riggs

Note: People following this should probably read this post on -patches
in the archive:
http://archives.postgresql.org/pgsql-patches/2006-02/msg00207.php

On Tue, Feb 14, 2006 at 05:20:55PM +0000, Simon Riggs wrote:
> Many patch submitters discover that they fall foul of various "you
> should have done"s at a late stage of the patch review process.

> I would like to suggest that we increase substantially the FAQ entries
> relating to patch submission. By we, I actually mean please could the
> committers sit down and agree some clarified written guidelines?

As I remember, there is a disinclination to increase the size of the
FAQ very much. This suggests maintaining it as a seperate document. Or
alternatively attach it as an appendix to the main documentation.

I liked your list BTW. It covers most of the common issues. I think you
missed SQL standards related issues. If you're submitting a patch to
increase SQL conformence, you need to say so.

> I believe if we do this we will have more patches produced, reviewed and
> committed from our available resources, as well as more hackers more
> regularly willing to face the challenges of getting a quality patch
> accepted. In the end we will live and die by the number of people
> submitting and how many of those go on to become regular contributors
> (should I say "serial hackers"?)

One real big issue is feedback. Some patches are obviously going to
receive much more feedback than others. But in some places there are
really very few people that can give meaningful feedback. I honestly
don't know what we can do about that other than try to grow the number
of people :(

Related to that is testing. I get the impression that very few patches
are tested by people other than the author before submission. This is
one thing the linux kernel does well. There exist trees that will take
almost any patch and people who download that and hammer it. Great for
testing stability before accepting it into the real tree.

Finally, several of the patches committed the last few days have been
fixing minor bugs or platform specific issues with various patches. One
thing that would be really nice is a real patch queue and have the
buildfarm machines occasionally apply one of the patches and try to run
with it. For people that don't have access to all sorts of
architechtures, it would be a great way of getting feedback on the
portability of the patch prior to actual submission to -HEAD.

Does give you ideas, doesn't it?

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Martijn van Oosterhout wrote:

>Finally, several of the patches committed the last few days have been
>fixing minor bugs or platform specific issues with various patches. One
>thing that would be really nice is a real patch queue and have the
>buildfarm machines occasionally apply one of the patches and try to run
>with it. For people that don't have access to all sorts of
>architechtures, it would be a great way of getting feedback on the
>portability of the patch prior to actual submission to -HEAD.
>
>
>

This would probably be fairly trivial to arrange. There is nothing
magical about the branches we build against on buildfarm - it just
happens to be HEAD and the REL_foo_STABLE branches. They are just names
in a config file.

If I had enough time there are all sorts of things like this I'd love to
set up. A fetchable url that says "try these experimental CVS branches"
or something like that would be great.

cheers

andrew

On Tue, 2006-02-14 at 16:17 -0500, Andrew Dunstan wrote:

> If I had enough time there are all sorts of things like this I'd love to
> set up. A fetchable url that says "try these experimental CVS branches"
> or something like that would be great.

How much time would you need? I think having every patch built before
anyone even looks at the code would sort out most of the issues I
mentioned.

I'm thinking in that direction for performance testing.

Best Regards, Simon Riggs

On Tue, Feb 14, 2006 at 09:54:12PM +0000, Simon Riggs wrote:
> On Tue, 2006-02-14 at 16:17 -0500, Andrew Dunstan wrote:
>
> > If I had enough time there are all sorts of things like this I'd love to
> > set up. A fetchable url that says "try these experimental CVS branches"
> > or something like that would be great.
>
> How much time would you need? I think having every patch built before
> anyone even looks at the code would sort out most of the issues I
> mentioned.

Indeed. I was thinking of downloading the pgbuildfarm code, setting up
an autoresponder for -patches and have it autocompile patches on
submission. I've never looked at the code so I have no idea how hard it
is to set it up, but it doesn't seem that difficult.

> I'm thinking in that direction for performance testing.

Is there any standard stuff (besides maybe pgbench) that could be run?

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> How much time would you need? I think having every patch built before
> anyone even looks at the code would sort out most of the issues I
> mentioned.

If I ran a buildfarm machine, I'd turn it off immediately if anyone
proposed setting up a system that would cause it to run code no one
had vetted... so I don't think the above will fly. It might or might
not be worth doing something with patches that have passed some kind
of initial review but aren't yet applied.

IMHO the thing we are really seriously short of is patch reviewers.
Neil and Bruce and I seem to be the only ones doing that much at all,
and the main burden is falling on Bruce. More eyeballs would help
much more than throwing machines at the problem.

regards, tom lane

On Tue, 2006-02-14 at 17:28 -0500, Tom Lane wrote:

> IMHO the thing we are really seriously short of is patch reviewers.
> Neil and Bruce and I seem to be the only ones doing that much at all,
> and the main burden is falling on Bruce. More eyeballs would help
> much more than throwing machines at the problem.

Well that was the basis of my original suggestion. Publish some
guidelines and everybody becomes a patch reviewer.

Best Regards, Simon Riggs

Tom Lane said:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
>> How much time would you need? I think having every patch built before
>> anyone even looks at the code would sort out most of the issues I
>> mentioned.
>
> If I ran a buildfarm machine, I'd turn it off immediately if anyone
> proposed setting up a system that would cause it to run code no one had
> vetted... so I don't think the above will fly. It might or might not
> be worth doing something with patches that have passed some kind of
> initial review but aren't yet applied.
>

Yes, I agree. Whast I had in mind was adding some sort of experimental
branch to CVS.

> IMHO the thing we are really seriously short of is patch reviewers.
> Neil and Bruce and I seem to be the only ones doing that much at all,
> and the main burden is falling on Bruce. More eyeballs would help much
> more than throwing machines at the problem.
>

Unfortunately, demands from my real job increased enormously right at the
time I was given commit privileges. I don't know when that will change.

People can review without having commit privs, though.

cheers

andrew

On Tuesday 14 February 2006 16:00, Martijn van Oosterhout wrote:
> > I would like to suggest that we increase substantially the FAQ entries
> > relating to patch submission. By we, I actually mean please could the
> > committers sit down and agree some clarified written guidelines?
>
> As I remember, there is a disinclination to increase the size of the
> FAQ very much. This suggests maintaining it as a seperate document. Or
> alternatively attach it as an appendix to the main documentation.
>

Huh? The current developers FAQ is at least 1/2 the size of the main FAQ. I
think adding a submission on patch submission guidelines is a great idea.
I'll have a patch based on Simon's post to -patches ready in the next 24
hours unless someone is really going to object.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

On Tue, 2006-02-14 at 22:54 +0000, Simon Riggs wrote:
> On Tue, 2006-02-14 at 17:28 -0500, Tom Lane wrote:
> > IMHO the thing we are really seriously short of is patch reviewers.
[...]
> Well that was the basis of my original suggestion. Publish some
> guidelines and everybody becomes a patch reviewer.

I agree guidelines would be help, but I hope (and doubt!) that is not
what is stopping people from reviewing patches. Anyone with the time and
inclination can review patches, guidelines or not -- reviewing patches
is actually a good way to learn more about Postgres internals. The
method I personally use for reviewing patches is trivial:

for each hunk in the patch
what is the intent of the hunk?
is there a better way to accomplish that?

(Actually applying the patch to a local tree and then browsing the tree
can be helpful to understand the context each hunk is modifying.)

Of course, the first few patches you review, you'll probably spend more
time on step 1 than on step 2, and you might not produce very many
useful review comments. But that's what practice is for :)

Newbie patch reviewers might also try reviewing patches for client
applications (e.g. psql, pg_dump) that do not require as much knowledge
of the rest of the source tree. If you are competent at C, you can
probably hack on psql/pg_dump/etc. with little additional knowledge.
Similarly, reviewing documentation patches is another easy way to get
involved -- SGML style fixes, spelling/grammar improvements and the like
require no knowledge of PG at all.

-Neil

On Tue, Feb 14, 2006 at 05:28:54PM -0500, Tom Lane wrote:
> Simon Riggs <simon(at)2ndquadrant(dot)com> writes:
> > How much time would you need? I think having every patch built before
> > anyone even looks at the code would sort out most of the issues I
> > mentioned.
>
> If I ran a buildfarm machine, I'd turn it off immediately if anyone
> proposed setting up a system that would cause it to run code no one
> had vetted... so I don't think the above will fly. It might or might
> not be worth doing something with patches that have passed some kind
> of initial review but aren't yet applied.

Ofcourse not totally unvetted code, but something like Bruce's patch
queue. Something that would compile them and tell you if they pass
regression, or even note when a patch no longer applied cleanly to
-HEAD. I was thinking it might be useful to have a level between
committer and just a regular person. Sort of like we don't trust this
guy to commit to -HEAD but enough to run basic tests on the patches.

> IMHO the thing we are really seriously short of is patch reviewers.
> Neil and Bruce and I seem to be the only ones doing that much at all,
> and the main burden is falling on Bruce. More eyeballs would help
> much more than throwing machines at the problem.

Yeah. Unfortunatly the parts of the code I am familiar with are not the
parts people submit patches on :(. There a lot of code there...

Have a nice day,
--
Martijn van Oosterhout <kleptog(at)svana(dot)org> http://svana.org/kleptog/
> Patent. n. Genius is 5% inspiration and 95% perspiration. A patent is a
> tool for doing 5% of the work and then sitting around waiting for someone
> else to do the other 95% so you can sue them.

On Tue, 2006-02-14 at 21:47 -0500, Neil Conway wrote:
> On Tue, 2006-02-14 at 22:54 +0000, Simon Riggs wrote:
> > On Tue, 2006-02-14 at 17:28 -0500, Tom Lane wrote:
> > > IMHO the thing we are really seriously short of is patch reviewers.
> [...]
> > Well that was the basis of my original suggestion. Publish some
> > guidelines and everybody becomes a patch reviewer.
>
> I agree guidelines would be help, but I hope (and doubt!) that is not
> what is stopping people from reviewing patches. Anyone with the time and
> inclination can review patches, guidelines or not

Yes, anyone can review patches, but will the patch submitter listen to
what has been said by the reviewer? Will a committer need to correct the
review comments?

If there is a park with a rule like "Keep Off the Grass" then it seems
most sensible to put up a sign that says that, rather than increase the
number of park keepers to explain the rules. Not everybody will take
notice of the sign, true, but it does allow non-park keepers to point
out that a guideline has not been followed. (Fairly sure that "KOtG"
should not be part of the PostgreSQL FAQ though).

[BTW, your patch reviewers guidelines were very good - FAQ also...]

Best Regards, Simon Riggs

xzilla(at)users(dot)sourceforge(dot)net (Robert Treat) writes:

> On Tuesday 14 February 2006 16:00, Martijn van Oosterhout wrote:
>> > I would like to suggest that we increase substantially the FAQ entries
>> > relating to patch submission. By we, I actually mean please could the
>> > committers sit down and agree some clarified written guidelines?
>>
>> As I remember, there is a disinclination to increase the size of the
>> FAQ very much. This suggests maintaining it as a seperate document. Or
>> alternatively attach it as an appendix to the main documentation.
>>
>
> Huh? The current developers FAQ is at least 1/2 the size of the main FAQ. I
> think adding a submission on patch submission guidelines is a great idea.
> I'll have a patch based on Simon's post to -patches ready in the next 24
> hours unless someone is really going to object.

If it were to be a new document, it would seem pretty sweet to call it
"The Hitchhiker's Guide To Getting Patches Accepted."

One of the "preface points" would be along the lines of...

"Here are some guidelines as to what things to do to make it as easy
as possible for proposed patches to be accepted with minimal change.
To not follow them all does not forcibly guarantee rejection; it just
increases the likelihood that the the amount of time and effort it
takes to handle it increases..."
--
select 'cbbrowne' || '@' || 'acm.org';
http://cbbrowne.com/info/spiritual.html
"When campaigning, be swift as the wind; in leisurely march, majestic
as the forest; in raiding and plundering, like fire; in standing, firm
as the mountains. As unfathomable as the clouds, move like a
thunderbolt." -- Sun Tzu, "The Art of War"

On Tuesday 14 February 2006 20:42, Robert Treat wrote:
> On Tuesday 14 February 2006 16:00, Martijn van Oosterhout wrote:
> > > I would like to suggest that we increase substantially the FAQ entries
> > > relating to patch submission. By we, I actually mean please could the
> > > committers sit down and agree some clarified written guidelines?
> >
> > As I remember, there is a disinclination to increase the size of the
> > FAQ very much. This suggests maintaining it as a seperate document. Or
> > alternatively attach it as an appendix to the main documentation.
>
> Huh? The current developers FAQ is at least 1/2 the size of the main FAQ.
> I think adding a submission on patch submission guidelines is a great idea.
> I'll have a patch based on Simon's post to -patches ready in the next 24
> hours unless someone is really going to object.

As stated, the following patch adds a list of patch submission guidelines
based on Simon Riggs suggestions to the developers FAQ.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> As stated, the following patch adds a list of patch submission guidelines
> based on Simon Riggs suggestions to the developers FAQ.

A couple minor comments ...

> ! <li>Ensure that your patch is generated against the most recent version
> ! of the code. If you are developing new features, this should be
> ! CVS HEAD; if it is a bug fix, this will be the most recent version of
> ! the branch which suffers from the bug. For more on branches in
> ! PostgreSQL, see <a href="#1.15">1.15</a>.</li>

Actually, I'd suggest working against HEAD in all cases; the committers
are used to adapting patches backwards, less so to adapting forwards.
(If a bug is fixed in newer releases and not older ones, there is
probably a good reason why not; so I don't see the point of encouraging
people to submit patches for bugs that only exist in older releases,
as this text seems to do.)

> ! <li>The patch should be generated in contextual diff format and should
> ! be applicable from the root directory. If you are unfamiliar with
> ! this, you might find the script <I>src/tools/makediff/difforig</I>
> ! useful. Unified diffs are only preferrable if the file changes are
> ! single-line changes and do not rely on the surrounding lines.</li>

I'd like the policy to be "contextual diffs are preferred", full stop.
Unidiffs are more compact but they sacrifice readability of the patch
(IMHO anyway) and when you are preparing a patch you should be thinking
first in terms of making it readable for the reviewers/committers.

Some things that follow along with the readability mandate, and should
be brought out somewhere here:
* avoid unnecessary whitespace changes. They just distract the
reviewer, and your formatting changes will probably not survive
the next pgindent run anyway.
* try to follow the project's code-layout conventions; again, this
makes it easier for the reviewer, and there's no long-term point
in trying to do it differently than pgindent would.

> ! <li>If your patch changes any existing defaults, you will need to
> ! explain why this is *required* or the patch will likely be rejected.
> ! New feature patches should also be accompanied by doc patches, and
> ! pointers to any relevant sections of the SQL standard are recommended
> ! as well. See <a href="#1.16">1.16</a> for more information on the
> ! SQL standards</li>

The above should be two items not one --- as written it downplays the
importance of providing documentation, which is something we must talk
up not down. (Bruce's original wording of the FAQ item I think
underplays it; we should absolutely not give the impression that
documentation is optional.) I'm not sticky about the docs being
properly-marked-up SGML, but I think you should at least have expended
the effort to explain what you are doing in English separate from the
code.

regards, tom lane

On Thursday 16 February 2006 00:27, Tom Lane wrote:
> Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > ! <li>The patch should be generated in contextual diff format and
> > should ! be applicable from the root directory. If you are unfamiliar
> > with ! this, you might find the script
> > <I>src/tools/makediff/difforig</I> ! useful. Unified diffs are only
> > preferrable if the file changes are ! single-line changes and do not
> > rely on the surrounding lines.</li>
>
> I'd like the policy to be "contextual diffs are preferred", full stop.
> Unidiffs are more compact but they sacrifice readability of the patch
> (IMHO anyway) and when you are preparing a patch you should be thinking
> first in terms of making it readable for the reviewers/committers.
>
> Some things that follow along with the readability mandate, and should
> be brought out somewhere here:
> * avoid unnecessary whitespace changes. They just distract the
> reviewer, and your formatting changes will probably not survive
> the next pgindent run anyway.

would diff -c --ignore-space-change be better?

> * try to follow the project's code-layout conventions; again, this
> makes it easier for the reviewer, and there's no long-term point
> in trying to do it differently than pgindent would.
>

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

Robert Treat wrote:
> On Thursday 16 February 2006 00:27, Tom Lane wrote:
> > Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > > ! <li>The patch should be generated in contextual diff format and
> > > should ! be applicable from the root directory. If you are unfamiliar
> > > with ! this, you might find the script
> > > <I>src/tools/makediff/difforig</I> ! useful. Unified diffs are only
> > > preferrable if the file changes are ! single-line changes and do not
> > > rely on the surrounding lines.</li>
> >
> > I'd like the policy to be "contextual diffs are preferred", full stop.
> > Unidiffs are more compact but they sacrifice readability of the patch
> > (IMHO anyway) and when you are preparing a patch you should be thinking
> > first in terms of making it readable for the reviewers/committers.
> >
> > Some things that follow along with the readability mandate, and should
> > be brought out somewhere here:
> > * avoid unnecessary whitespace changes. They just distract the
> > reviewer, and your formatting changes will probably not survive
> > the next pgindent run anyway.
>
> would diff -c --ignore-space-change be better?

No, because some whitespace changes are important. For example when you
indent a piece of code one level higher. The submitter should eyeball
the patch (in diff form) and clean things up when something unexpected
appears, like a no-op whitespace change.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> On Thursday 16 February 2006 00:27, Tom Lane wrote:
>> * avoid unnecessary whitespace changes. They just distract the
>> reviewer, and your formatting changes will probably not survive
>> the next pgindent run anyway.

> would diff -c --ignore-space-change be better?

Hmm. Not sure --- there are situations where whitespace *does* matter,
so having that as a blanket policy doesn't seem wise. Also I'm worried
that a diff made this way would confuse patch (for instance, because
line numbers following an omitted whitespace change wouldn't match up).
Probably best not to go there, but just focus on the point about keeping
the patch readable.

regards, tom lane

On Thursday 16 February 2006 00:27, Tom Lane wrote:
> Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > As stated, the following patch adds a list of patch submission guidelines
> > based on Simon Riggs suggestions to the developers FAQ.
>
> A couple minor comments ...
>

Attached patch updated based on previous feedback.

--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL

On Thu, 2006-02-16 at 15:09 -0500, Robert Treat wrote:
> On Thursday 16 February 2006 00:27, Tom Lane wrote:
> > Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > > As stated, the following patch adds a list of patch submission guidelines
> > > based on Simon Riggs suggestions to the developers FAQ.
> >
> > A couple minor comments ...
> >
>
> Attached patch updated based on previous feedback.

Looks like a useful addition to me and I'll do my best to follow it.

Best Regards, Simon Riggs

Tom Lane wrote:
> > ! <li>The patch should be generated in contextual diff format and should
> > ! be applicable from the root directory. If you are unfamiliar with
> > ! this, you might find the script <I>src/tools/makediff/difforig</I>
> > ! useful. Unified diffs are only preferrable if the file changes are
> > ! single-line changes and do not rely on the surrounding lines.</li>
>
> I'd like the policy to be "contextual diffs are preferred", full stop.
> Unidiffs are more compact but they sacrifice readability of the patch
> (IMHO anyway) and when you are preparing a patch you should be thinking
> first in terms of making it readable for the reviewers/committers.

This unified diff sentence was added recently, because I had a case
where I was posting a diff, and a unified version was actually clearer
than the context diff version because it was a file were we were
changing discrete lines, rather than blocks of code. It might be a
small enough number of cases that it isn't worth mentioning, but we have
had people say they find unified diffs clearer, so I wanted to mention
_where_ unified diffs are clearer, and where they are not. I thought
this might encourage people to use content diffs more often if they
understood _why_?

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > On Thursday 16 February 2006 00:27, Tom Lane wrote:
> >> * avoid unnecessary whitespace changes. They just distract the
> >> reviewer, and your formatting changes will probably not survive
> >> the next pgindent run anyway.
>
> > would diff -c --ignore-space-change be better?
>
> Hmm. Not sure --- there are situations where whitespace *does* matter,
> so having that as a blanket policy doesn't seem wise. Also I'm worried
> that a diff made this way would confuse patch (for instance, because
> line numbers following an omitted whitespace change wouldn't match up).
> Probably best not to go there, but just focus on the point about keeping
> the patch readable.

Agreed. It isn't that we don't want whitespace changes, just that we
don't want insignificant whitespace changes.

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +

Robert Treat wrote:
> On Thursday 16 February 2006 00:27, Tom Lane wrote:
> > Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> writes:
> > > As stated, the following patch adds a list of patch submission guidelines
> > > based on Simon Riggs suggestions to the developers FAQ.
> >
> > A couple minor comments ...
> >
>
> Attached patch updated based on previous feedback.

I have applied your patch (attached) after some cleanup, and applied to
8.1.X too. The only part I removed was the requirement to research
previous discussion of the patch. That usually is not an issue, and can
always be requested after the patch is submitted.

Thanks, nice improvement.

--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com

+ If your life is a hard drive, Christ can be your backup. +

On Tue, 14 Feb 2006 21:54:12 +0000
Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:

> On Tue, 2006-02-14 at 16:17 -0500, Andrew Dunstan wrote:
>
> > If I had enough time there are all sorts of things like this I'd love to
> > set up. A fetchable url that says "try these experimental CVS branches"
> > or something like that would be great.
>
> How much time would you need? I think having every patch built before
> anyone even looks at the code would sort out most of the issues I
> mentioned.

Sorry I've gotten into this late. The PLM developed at OSDL might be
useful here. We're still grabbing daily snapshots from CVS and patches
could be submitted against those to see if they apply cleanly. Sparse
is also run but no compiling is done, although that could be easily
arranged.

Here a link:
http://plm.osdl.org/plm-cgi/plm

Mark