[Mesa-dev] [PATCH 2/3] docs: update documentation about patch formatting, testing, etc
imirkin at alum.mit.edu
Mon May 25 09:33:40 PDT 2015
On Mon, May 25, 2015 at 12:20 PM, Brian Paul <brianp at vmware.com> wrote:
> docs/devinfo.html | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 102 insertions(+), 2 deletions(-)
> diff --git a/docs/devinfo.html b/docs/devinfo.html
> index c7e4171..a6fb76b 100644
> --- a/docs/devinfo.html
> +++ b/docs/devinfo.html
> @@ -124,13 +124,113 @@ src/mesa/state_tracker/st_glsl_to_tgsi.cpp can serve as examples.
> <h2 id="submitting">Submitting patches</h2>
> -You should always run the Mesa Testsuite before submitting patches.
> -The Testsuite can be run using the 'make check' command. All tests
> +The basic guidelines for submitting patches are:
> +<li>Patches should be sufficiently tested before submitting.
> +<li>Code patches should follow Mesa coding conventions.
> +<li>Whenever possible, patches should only effect individual Mesa/Gallium
> +<li>Patches should never introduce build breaks and should be bisectable (see
> +<code>git bisect</code>.)
> +<li>Patches should be properly formatted (see below).
> +<li>Patches should be submitted to mesa-dev for review using
> +<code>git send-email</code>.
> +<li>Patches should not mix code changes with code formatting changes (except,
> +perhaps, in very trivial cases.)
> +<h3>Patch formatting</h3>
> +The basic rules for patch formatting are:
> +<li>Lines should be limited to 75 characters or less so that git logs
> +displayed in 80-column terminals avoid line wrapping. Note that git
> +log uses 4 spaces of indentation (4 + 75 < 80).
> +<li>The first line should be a short, concise summary of the change prefixed
> +with a module name. Examples:
> + mesa: Add support for querying GL_VERTEX_ATTRIB_ARRAY_LONG
> + gallium: add PIPE_CAP_DEVICE_RESET_STATUS_QUERY
> + i965: Fix missing type in local variable declaration.
> +<li>Subsequent patch comments should describe the change in more detail,
> +if needed. For example:
> + i965: Remove end-of-thread SEND alignment code.
> + This was present in Eric's initial implementation of the compaction code
> + for Sandybridge (commit 077d01b6). There is no documentation saying this
> + is necessary, and removing it causes no regressions in piglit on any
> + platform.
> +<li>A "Signed-off-by:" line is not required, but not discouraged either.
> +<li>If a patch address a bugzilla issue, that should be noted in the
> +patch comment. For example:
> + Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
> +<li>If there have been several revisions to a patch during the review
> +process, they should be noted such as in this example:
> + st/mesa: add ARB_texture_stencil8 support (v4)
> + if we support stencil texturing, enable texture_stencil8
> + there is no requirement to support native S8 for this,
> + the texture can be converted to x24s8 fine.
> + v2: fold fixes from Marek in:
> + a) put S8 last in the list
> + b) fix renderable to always test for d/s renderable
> + fixup the texture case to use a stencil only format
> + for picking the format for the texture view.
> + v3: hit fallback for getteximage
> + v4: put s8 back in front, it shouldn't get picked now (Ilia)
Is there really value in including things like this in the commit log?
I always put this info, along with any comments, below the --- in the
emails I send, which is also the linux kernel way of doing things.
When you look at the commit 5 years from now, will "v3: hit fallback
for getteximage" really make sense and/or be valuable?
> +<li>If someone tested your patch, document it with a line like this:
> + Tested-by: Joe Hacker <jhacker at foo.com>
I think you meant < and > (and below)
> +<li>If the patch was reviewed (usually the case) or acked by someone,
> +that should be documented with:
> + Reviewed-by: Joe Hacker <jhacker at foo.com>
> + Acked-by: Joe Hacker <jhacker at foo.com>
> +<h3>Testing Patches</h3>
> +It should go without saying that patches must be tested. In general,
> +do whatever testing is prudent.
> +You should always run the Mesa test suite before submitting patches.
> +The test suite can be run using the 'make check' command. All tests
> must pass before patches will be accepted, this may mean you have
> to update the tests themselves.
> +Whenever possible and applicable, test the patch with
> +<a href="http://people.freedesktop.org/~nh/piglit/">Piglit</a> to
> +check for regressions.
> +<h3>Mailing Patches</h3>
> Patches should be sent to the Mesa mailing list for review.
> When submitting a patch make sure to use git send-email rather than attaching
> patches to emails. Sending patches as attachments prevents people from being
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
More information about the mesa-dev