[Mesa-dev] [PATCH 2/3] docs: update documentation about patch formatting, testing, etc
Brian Paul
brianp at vmware.com
Mon May 25 09:44:26 PDT 2015
On 05/25/2015 10:33 AM, Ilia Mirkin wrote:
> 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>
>>
>> <p>
>> -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:
>> +</p>
>> +
>> +<ul>
>> +<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
>> +components.
>> +<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.)
>> +</ul>
>> +
>> +<h3>Patch formatting</h3>
>> +
>> +<p>
>> +The basic rules for patch formatting are:
>> +</p>
>> +
>> +<ul>
>> +<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).
>
> <
Thanks. I'm an infrequent html write.
>
>> +<li>The first line should be a short, concise summary of the change prefixed
>> +with a module name. Examples:
>> +<pre>
>> + 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.
>> +</pre>
>> +<li>Subsequent patch comments should describe the change in more detail,
>> +if needed. For example:
>> +<pre>
>> + 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.
>> +</pre>
>> +<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:
>> +<pre>
>> + Bugzilla: https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.freedesktop.org_show-5Fbug.cgi-3Fid-3D89689&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=eJBsLPHgfqQUObvZlLo6hv_Sn4frMm45eK5WAcvnRhI&s=Qct33vrigM7IEYKI3f2cPLDvVPQrpjs47wF9Aew9as0&e=
>> +</pre>
>> +<li>If there have been several revisions to a patch during the review
>> +process, they should be noted such as in this example:
>> +<pre>
>> + 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?
TBH, I don't have a strong opinion on this. Sometimes the revisions
might relate one's thought process, but in the end the intermediate
steps usually don't matter. The example above seems to be the current
consensus.
>
>> +</pre>
>> +<li>If someone tested your patch, document it with a line like this:
>> +<pre>
>> + Tested-by: Joe Hacker <jhacker at foo.com>
>
> I think you meant < and > (and below)
I'll fix those before committing.
-Brian
>
>> +</pre>
>> +<li>If the patch was reviewed (usually the case) or acked by someone,
>> +that should be documented with:
>> +<pre>
>> + Reviewed-by: Joe Hacker <jhacker at foo.com>
>> + Acked-by: Joe Hacker <jhacker at foo.com>
>> +</pre>
>> +</ul>
>> +
>> +
>> +
>> +<h3>Testing Patches</h3>
>> +
>> +<p>
>> +It should go without saying that patches must be tested. In general,
>> +do whatever testing is prudent.
>> +</p>
>> +
>> +<p>
>> +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.
>> </p>
>>
>> <p>
>> +Whenever possible and applicable, test the patch with
>> +<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__people.freedesktop.org_-7Enh_piglit_&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=eJBsLPHgfqQUObvZlLo6hv_Sn4frMm45eK5WAcvnRhI&s=gK6KUqtQZWTATDpk-6UwMASmuhOi7cRwpXQ6YEC-Gm4&e= ">Piglit</a> to
>> +check for regressions.
>> +</p>
>> +
>> +
>> +<h3>Mailing Patches</h3>
>> +
>> +<p>
>> 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
>> --
>> 1.9.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=eJBsLPHgfqQUObvZlLo6hv_Sn4frMm45eK5WAcvnRhI&s=6rORknGW35QNUY-S5XICVNXoH6JWa047E-As302o9Lo&e=
More information about the mesa-dev
mailing list