docs/submittingpatches.html: rework the #criteria section

Reword the section to focus on what is allowed, using a more brief, yet
descriptive wording.

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>
This commit is contained in:
Emil Velikov 2017-02-13 19:23:38 +00:00 committed by Emil Velikov
parent af9a4d9005
commit d7e0ff0e2b

View File

@ -259,15 +259,53 @@ Thus, drop the line <strong>only</strong> if you want to cancel the nomination.
<h2 id="criteria">Criteria for accepting patches to the stable branch</h2>
Mesa has a designated release manager for each stable branch, and the release
manager is the only developer that should be pushing changes to these
branches. Everyone else should simply nominate patches using the mechanism
described above.
manager is the only developer that should be pushing changes to these branches.
Everyone else should nominate patches using the mechanism described above.
The stable-release manager will work with the list of nominated patches, and
for each patch that meets the criteria below will cherry-pick the patch with:
<code>git cherry-pick -x &lt;commit&gt;</code>. The <code>-x</code> option is
important so that the picked patch references the commit ID of the original
patch.
The following rules define which patches are accepted and which are not. The
stable-release manager is also given broad discretion in rejecting patches
that have been nominated.
<ul>
<li>Patch must conform with the <a href="#guidelines">Basic guidelines</a></li>
<li>Patch must have landed in master first. In case where the original
patch is too large and/or otherwise contradicts with the rules set within, a
backport is appropriate.</li>
<li>It must not introduce a regression - be that build or runtime wise.
Note: If the regression is due to faulty piglit/dEQP/CTS/other test the
latter must be fixed first. A reference to the offending test(s) and
respective fix(es) should be provided in the nominated patch.</li>
<li>Patch cannot be larger than 100 lines.</li>
<li>Patches that move code around with no functional change should be
rejected.</li>
<li>Patch must be a bug fix and not a new feature.
Note: An exception to this rule, are hardware-enabling "features". For
example, backports of new code to support a newly-developed hardware product
can be accepted if they can be reasonably determined not to have effects on
other hardware.</li>
<li>Patch must be reviewed, For example, the commit message has Reviewed-by,
Signed-off-by, or Tested-by tags from someone but the author.</li>
<li>Performance patches are considered only if they provide information
about the hardware, program in question and observed improvement. Use numbers
to represent your measurements.</li>
</ul>
If the patch complies with the rules it will be
<a href="releasing.html#pickntest">cherry-picked</a>. Alternatively the release
manager will reply to the patch in question stating why the patch has been
rejected or would request a backport.
A summary of all the picked/rejected patches will be presented in the
<a href="releasing.html#prerelease">pre-release</a> announcement.
The stable-release manager may at times need to force-push changes to the
stable branches, for example, to drop a previously-picked patch that was later
@ -275,72 +313,6 @@ identified as causing a regression). These force-pushes may cause changes to
be lost from the stable branch if developers push things directly. Consider
yourself warned.
The stable-release manager is also given broad discretion in rejecting patches
that have been nominated for the stable branch. The most basic rule is that
the stable branch is for bug fixes only, (no new features, no
regressions). Here is a non-exhaustive list of some reasons that a patch may
be rejected:
<ul>
<li>Patch introduces a regression. Any reported build breakage or other
regression caused by a particular patch, (game no longer works, piglit test
changes from PASS to FAIL), is justification for rejecting a patch.</li>
<li>Patch is too large, (say, larger than 100 lines)</li>
<li>Patch is not a fix. For example, a commit that moves code around with no
functional change should be rejected.</li>
<li>Patch fix is not clearly described. For example, a commit message
of only a single line, no description of the bug, no mention of bugzilla,
etc.</li>
<li>Patch has not obviously been reviewed, For example, the commit message
has no Reviewed-by, Signed-off-by, nor Tested-by tags from anyone but the
author.</li>
<li>Patch has not already been merged to the master branch. As a rule, bug
fixes should never be applied first to a stable branch. Patches should land
first on the master branch and then be cherry-picked to a stable
branch. (This is to avoid future releases causing regressions if the patch
is not also applied to master.) The only things that might look like
exceptions would be backports of patches from master that happen to look
significantly different.</li>
<li>Patch depends on too many other patches. Ideally, all stable-branch
patches should be self-contained. It sometimes occurs that a single, logical
bug-fix occurs as two separate patches on master, (such as an original
patch, then a subsequent fix-up to that patch). In such a case, these two
patches should be squashed into a single, self-contained patch for the
stable branch. (Of course, if the squashing makes the patch too large, then
that could be a reason to reject the patch.)</li>
<li>Patch includes new feature development, not bug fixes. New OpenGL
features, extensions, etc. should be applied to Mesa master and included in
the next major release. Stable releases are intended only for bug fixes.
Note: As an exception to this rule, the stable-release manager may accept
hardware-enabling "features". For example, backports of new code to support
a newly-developed hardware product can be accepted if they can be reasonably
determined not to have effects on other hardware.</li>
<li>Patch is a performance optimization. As a rule, performance patches are
not candidates for the stable branch. The only exception might be a case
where an application's performance was recently severely impacted so as to
become unusable. The fix for this performance regression could then be
considered for a stable branch. The optimization must also be
non-controversial and the patches still need to meet the other criteria of
being simple and self-contained</li>
<li>Patch introduces a new failure mode (such as an assert). While the new
assert might technically be correct, for example to make Mesa more
conformant, this is not the kind of "bug fix" we want in a stable
release. The potential problem here is that an OpenGL program that was
previously working, (even if technically non-compliant with the
specification), could stop working after this patch. So that would be a
regression that is unacceptable for the stable branch.</li>
</ul>
<h2 id="gittips">Git tips</h2>
<ul>