Re: Free WAL caches on switching segments - Mailing list pgsql-patches

On Mon, 2006-02-13 at 23:33 -0500, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Yes, your vote counts very much.  What if I apply the patch, but mark
> > the posix_advise() call in a NOT_USED macro block, so it will be ready
> > for people to test, but will not be used until we are sure.
>
> Sounds like a recipe for ensuring it never will be tested.  What's
> needed here is some actual tests, not preparation...

Without discussing this particular patch, IMHO we need a clear checklist
of items that are required before a patch is accepted onto the patches
awaiting application list.

We seem to have had a few patches rejected on grounds that could have
been picked up by other people, not just Tom and Bruce. This wastes
everybody's time because the patch writer might have fixed whatever was
wrong with it a while back if the patch had been rejected earlier. It
will also eventually bring the process into disrepute if one accepts
patches onto the queue and then another raises major objections that
could easily have been rectified earlier.

So let's agree a checklist beforehand. That way patch submitters can be
told to resubmit much earlier by other list watchers, with less wasted
time (elapsed and from core folk) and perhaps a gentler experience for
first-time submitters. Control can and should still lie with committers.

Suggested checklist:
1. has patch been discussed previously? Y/N
- if Y, give direct link to archive of message, and/or bug#
- if N discuss on appropriate list, or expect to be rejected
2. patch target: cvstip or stated branch(es)
3. patch application location: root directory only
4. patch format: diff -c only
5. confirm licence is BSD: Y/N
6. port specific: Y/N, if Y list ports
7. confirm passes make check (on listed ports)
8. provide implementation overview, preferably in code comments
9. if it is a performance patch, provide confirming test results. It is
OK to post patches without these, though the patch will not be applied
until *somebody* has tested the patches and found a valuable performance
effect directly attributable to the patch.
10. If it is a new feature patch, confirm that it has been tested for
all desired scenarios. If it has not, this should be clearly stated as a
request for a particular kind of test to be performed. Note that the
patch will go no further until that test has been performed.
11. if it is a new feature patch, does it break any existing defaults?
Explain why this is *required* or patch will be rejected. New feature
patches should be accompanied by doc patches also.
12. Even if you pass all of the above, the patch may still be rejected
for other technical reasons. You should be prepared to listen to
comments received and perform any agreed rework. Even if you have
received positive comments from some community members, others may spot
problems with your approach, coding style or many other issues.
13. Successful patches will be notified to you by email and you will be
credited for that work in the next set of release notes.

I would also suggest that we have two patch queues:
- patches awaiting performance testing
- patches awaiting application (current one)

That way anybody wanting to test new performance add-ons can do so and
reply to the list with confirmation that the patch can now be added to
the second (main) list of patches.

Of course, this suggestion will be immediately rejected because it
wasn't discussed on -hackers ;-)

Best Regards, Simon Riggs