Robert Treat <[EMAIL PROTECTED]> 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
---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
