Eric Engestrom <eric.engest...@intel.com> writes: > On Wednesday, 2018-11-28 13:36:29 -0800, Eric Anholt wrote: >> Jordan Justen <jordan.l.jus...@intel.com> writes: >> >> > This documents a mechanism for using GitLab Merge Requests as an >> > optional, secondary way to get code reviews for patchsets. >> > >> > We still require all patches to be emailed. >> > >> > Aside from the potential usage for code review comments, it might also >> > help reviewers to find unmerged patchsets which need review. (Assuming >> > it doesn't fall into the same fate of patchwork with hundreds of open >> > MRs.) >> > >> > Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com> >> > Cc: Jason Ekstrand <ja...@jlekstrand.net> >> > --- >> > docs/submittingpatches.html | 25 +++++++++++++++++++++++++ >> > 1 file changed, 25 insertions(+) >> > >> > diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html >> > index 5d8ba443191..852f28c198a 100644 >> > --- a/docs/submittingpatches.html >> > +++ b/docs/submittingpatches.html >> > @@ -24,6 +24,7 @@ >> > <li><a href="#testing">Testing Patches</a> >> > <li><a href="#mailing">Mailing Patches</a> >> > <li><a href="#reviewing">Reviewing Patches</a> >> > +<li><a href="#gitlab">GitLab Merge Requests</a> >> > <li><a href="#nominations">Nominating a commit for a stable branch</a> >> > <li><a href="#criteria">Criteria for accepting patches to the stable >> > branch</a> >> > <li><a href="#backports">Sending backports for the stable branch</a> >> > @@ -282,6 +283,30 @@ which tells the patch author that the patch can be >> > committed, as long >> > as the issues are resolved first. >> > </p> >> > >> > +<h2 id="gitlab">GitLab Merge Requests</h2> >> > + >> > +<p> >> > + <a href="https://gitlab.freedesktop.org/mesa/mesa";>GitLab</a> Merge >> > + Requests (MR) can be used as an optional, secondary method of >> > + obtaining code review for patches. >> > +</p> >> > + >> > +<ul> >> > + <li>All patches should be submitted using email as well >> >> Like others, I disagree and think this will lead to confusion. Let >> people send to one or the other. >> >> > + <li>Consider including a link to the MR in your email based >> > + cover-letter >> > + <li>Address code review from both email and the MR >> > + <li>Add appropriate labels to your MR. For example: >> > + <ul> >> > + <li>Mesa changes affect all drivers: mesa >> > + <li>Hardware vendor specific code: amd, intel, nvidia, etc >> > + <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi, >> > radv, vc4, etc >> > + <li>Other tag examples: gallium, util >> > + </ul> >> > + <li>Never use the merge button on the GitLab page to push patches >> >> Why "never use the merge button"? We've been using rebase+merge in >> xserver and it's *awesome* and has greatly increased the review rate. > > Because GitLab doesn't (yet) support tracking 'reviewed' status in > the commit message, which is something I think is important to track for > various reasons. If those are manually added to the commit messages > before the MR is merged, then there's no issue there. > > We could also have a check that an MR can't be merged until all its > commits contain the string "Reviewed-by:", which might be a better > solution?
In the xserver we decided that it was more important to have MRs be available (so a reviewer can click "merge when the tests are green") than to have in-git tracking of whether commits were reviewed. I think it's been a great tradeoff, but I understand if Mesa doesn't want to make the same one.
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
