diff options
author | Emil Velikov <[email protected]> | 2017-02-13 19:23:38 +0000 |
---|---|---|
committer | Emil Velikov <[email protected]> | 2017-02-20 18:21:22 +0000 |
commit | d7e0ff0e2b2e501e58b53464040af7c39688263a (patch) | |
tree | 0fee7cfe75ae61127d51ddee6597a624aee5660a | |
parent | af9a4d900527f4c1efeec7c4eb2cb8a602f9a9cb (diff) |
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 <[email protected]>
Reviewed-by: Nicolai Hähnle <[email protected]>
-rw-r--r-- | docs/submittingpatches.html | 120 |
1 files changed, 46 insertions, 74 deletions
diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html index f8380b0a542..ef5be716d93 100644 --- a/docs/submittingpatches.html +++ b/docs/submittingpatches.html @@ -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 <commit></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> |