[Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)

Discussion:

Jordan Justen

2018-12-05 23:32:05 UTC

This documents a process for using GitLab Merge Requests as an second
way to submit code changes for Mesa. Only one of the two methods is
allowed for each patch series.

We will *not* require all patches to be emailed. Some code changes may
be reviewed and merged without any discussion on the mesa-dev email
list.

v2:
* No longer require email. Allow submitter to choose email or a
GitLab merge request.
* Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
Matt, Michel and Rob.

Signed-off-by: Jordan Justen <***@intel.com>
---
docs/submittingpatches.html | 76 ++++++++++++++++++++++++++++++++++---
1 file changed, 71 insertions(+), 5 deletions(-)

diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 92d954a2d09..21175988d0b 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -21,7 +21,7 @@
<li><a href="#guidelines">Basic guidelines</a>
<li><a href="#formatting">Patch formatting</a>
<li><a href="#testing">Testing Patches</a>
-<li><a href="#mailing">Mailing Patches</a>
+<li><a href="#submit">Submitting Patches</a>
<li><a href="#reviewing">Reviewing Patches</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>
@@ -42,8 +42,10 @@ components.
<code>git bisect</code>.)
<li>Patches should be properly <a href="#formatting">formatted</a>.
<li>Patches should be sufficiently <a href="#testing">tested</a> before submitting.
-<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
-for <a href="#reviewing">review</a> using <code>git send-email</code>.
+<li>Patches should be <a href="#submit">submitted</a>
+to <a href="#mailing">mesa-dev</a> or with
+a <a href="#merge-request">merge request</a>
+for <a href="#reviewing">review</a>.

</ul>

@@ -180,10 +182,19 @@ run.
</p>

-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>

<p>
-Patches should be sent to the mesa-dev mailing list for review:
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your changes.
+</p>
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
+Patches may be sent to the mesa-dev mailing list for review:
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
mesa-***@lists.freedesktop.org</a>.
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you may need to contact
your email administrator for this.)
</p>

+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.
+</p>
+<p>
+ Add labels to your MR to help reviewers find it. For example:
+ <ul>
+ <li>Mesa changes affecting all drivers: mesa
+ <li>Hardware vendor specific code: amd, intel, nvidia, ...
+ <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
+ radv, vc4, ...
+ <li>Other tag examples: gallium, util
+ </ul>
+</p>
+<p>
+ If you revise your patches based on code review and push an update
+ to your branch, you should maintain a <strong>clean</strong> history
+ in your patches. There should not be "fixup" patches in the history.
+ The series should be buildable and functional after every commit
+ whenever you push the branch.
+</p>
+<p>
+ It is your responsibility to keep the MR alive and making progress,
+ as there are no guarantees that a Mesa dev will independently take
+ interest in it.
+</p>
+<p>
+ Some other notes:
+ <ul>
+ <li>Make changes and update your branch based on feedback
+ <li>Old, stale MR may be closed, but you can reopen it if you
+ still want to pursue the changes
+ <li>You should periodically check to see if your MR needs to be
+ rebased
+ <li>Make sure your MR is closed if your patches get pushed outside
+ of GitLab
+ </ul>
+</p>
+
<h2 id="reviewing">Reviewing Patches</h2>

+<p>
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
+ Requests</a> page.
+</p>
+
<p>
When you've reviewed a patch on the mailing list, please be unambiguous
about your review. That is, state either

--
2.20.0.rc2

Jason Ekstrand

2018-12-05 23:44:18 UTC

Permalink

Do we want to require a cover-letter to be sent to the ML? Ideally, we'd
just have a bot that makes one every time someone submits a MR and sends it
to the list. Maybe someone just needs to write that bot.

Post by Jordan Justen
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you
may need to contact
your email administrator for this.)
</p>
+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.
+</p>
+<p>
+ <ul>
+ <li>Mesa changes affecting all drivers: mesa
+ <li>Hardware vendor specific code: amd, intel, nvidia, ...
+ <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
+ radv, vc4, ...
+ <li>Other tag examples: gallium, util
+ </ul>
+</p>
+<p>
+ If you revise your patches based on code review and push an update
+ to your branch, you should maintain a <strong>clean</strong> history
+ in your patches. There should not be "fixup" patches in the history.
+ The series should be buildable and functional after every commit
+ whenever you push the branch.
+</p>
+<p>
+ It is your responsibility to keep the MR alive and making progress,
+ as there are no guarantees that a Mesa dev will independently take
+ interest in it.
+</p>
+<p>
+ <ul>
+ <li>Make changes and update your branch based on feedback
+ <li>Old, stale MR may be closed, but you can reopen it if you
+ still want to pursue the changes
+ <li>You should periodically check to see if your MR needs to be
+ rebased
+ <li>Make sure your MR is closed if your patches get pushed outside
+ of GitLab
+ </ul>
+</p>
+
<h2 id="reviewing">Reviewing Patches</h2>
+<p>
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests
">Merge
+ Requests</a> page.
+</p>
+
<p>
When you've reviewed a patch on the mailing list, please be unambiguous
about your review. That is, state either
--
2.20.0.rc2
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Jordan Justen

2018-12-06 01:05:29 UTC

Permalink

Post by Jason Ekstrand

Post by Jordan Justen
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
<p>
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your changes.
+</p>

Regarding the cover-letter, I put in this weasel worded sentence.
Should it instead say this is required and that it should be a git
format-patch generated cover letter?

Or, should we drop it entirely and assume we'll get an automated way
to send an email to the list whenever a new MR is opened?

Relatedly, I think it might be possible to enable an irc channel to be
notified about pushes and MR's. Not sure if it'd be a good idea, or
maybe too noisy.

-Jordan

Jason Ekstrand

2018-12-06 03:46:00 UTC

Permalink

Post by Jordan Justen

Post by Jason Ekstrand

Do we want to require a cover-letter to be sent to the ML? Ideally, we'd
just have a bot that makes one every time someone submits a MR and sends

Post by Jason Ekstrand
to the list. Maybe someone just needs to write that bot.

you

Post by Jason Ekstrand

Post by Jordan Justen
may need to contact
your email administrator for this.)
</p>
+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.

Regarding the cover-letter, I put in this weasel worded sentence.
Should it instead say this is required and that it should be a git
format-patch generated cover letter?

I didn't read far enough.... No, I don't think we need to require
git-send-email formatted.

Post by Jordan Justen
Or, should we drop it entirely and assume we'll get an automated way
to send an email to the list whenever a new MR is opened?
Relatedly, I think it might be possible to enable an irc channel to be
notified about pushes and MR's. Not sure if it'd be a good idea, or
maybe too noisy.

We should totally have an IRC bot. We had one for wayland and weston when
I was working on those and it was great. If it notifies us of every
change, it may be too much but if it dumps something in the channel for
every new MR, that shouldn't be bad at all.

Also, regardless of the weasel-wording, this is

Acked-by: Jason Ekstrand <***@jlekstrand.net>

--Jason

Erik Faye-Lund

2018-12-07 09:19:23 UTC

Permalink

On Wed, Dec 5, 2018 at 7:05 PM Jordan Justen <

Post by Jordan Justen

On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen <

Post by Jordan Justen
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
<p>
-Patches should be sent to the mesa-dev mailing list for
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your

changes.

Post by Jordan Justen
+</p>

Do we want to require a cover-letter to be sent to the ML?

Ideally, we'd

just have a bot that makes one every time someone submits a MR

and sends it

to the list. Maybe someone just needs to write that bot.

Post by Jordan Justen
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="

https://lists.freedesktop.org/mailman/listinfo/mesa-dev">

Post by Jordan Justen
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches.

(Note that you

Merge

Post by Jordan Justen
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community,

you can

Post by Jordan Justen
+ send an email to the mesa-dev email list including a link to

the MR.

Post by Jordan Justen
+ Don't send the patch to mesa-dev, just the MR link.

Regarding the cover-letter, I put in this weasel worded sentence.
Should it instead say this is required and that it should be a git
format-patch generated cover letter?

I didn't read far enough.... No, I don't think we need to require
git-send-email formatted.

We should totally have an IRC bot. We had one for wayland and weston
when I was working on those and it was great. If it notifies us of
every change, it may be too much but if it dumps something in the
channel for every new MR, that shouldn't be bad at all.

Automated emails (and perhaps IRC bot) would be really nice. Even
better if it could be hooked up to scripts/get_reviewer.pl, and
automatically CC "the right people". Perhaps a we can also somehow map
emails to irc nicks?

Eric Engestrom

2018-12-07 18:15:23 UTC

Permalink

Post by Erik Faye-Lund

On Wed, Dec 5, 2018 at 7:05 PM Jordan Justen <

Post by Jordan Justen

On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen <

changes.

Post by Jordan Justen
+</p>

Do we want to require a cover-letter to be sent to the ML?

Ideally, we'd

just have a bot that makes one every time someone submits a MR

and sends it

to the list. Maybe someone just needs to write that bot.

Post by Jordan Justen
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="

https://lists.freedesktop.org/mailman/listinfo/mesa-dev">

Post by Jordan Justen
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches.

(Note that you

Merge

Post by Jordan Justen
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community,

you can

Post by Jordan Justen
+ send an email to the mesa-dev email list including a link to

the MR.

Post by Jordan Justen
+ Don't send the patch to mesa-dev, just the MR link.

Regarding the cover-letter, I put in this weasel worded sentence.
Should it instead say this is required and that it should be a git
format-patch generated cover letter?

I didn't read far enough.... No, I don't think we need to require
git-send-email formatted.

We should totally have an IRC bot. We had one for wayland and weston
when I was working on those and it was great. If it notifies us of
every change, it may be too much but if it dumps something in the
channel for every new MR, that shouldn't be bad at all.

Automated emails (and perhaps IRC bot) would be really nice.

Agreed. Email would be great to help with the transition.
There's work currently being done on GitLab to allow for mailing lists
to be notified; this should cover 'new MR' as well.
If we need this feature before GitLab is done, it should be possible to
write a bot using the webhooks, just needs someone to take the time to
do it :)

For IRC, there's already some integration, but it's limited to notifying
about git pushes for now:
https://docs.gitlab.com/ee/user/project/integrations/irker.html

There's an open issue about adding more events, but it hasn't seen much
activity:
https://gitlab.com/gitlab-org/gitlab-ce/issues/7965

Post by Erik Faye-Lund
Even better if it could be hooked up to scripts/get_reviewer.pl, and
automatically CC "the right people".

Side note, I've been rewriting that script, although I need to send v2
out at some point:
https://patchwork.freedesktop.org/patch/226256/

I would be trivial to hook that into a bot we'd write, but I don't think
GitLab has support for something like this. I just opened an issue about
adding support directly in GitLab:
https://gitlab.com/gitlab-org/gitlab-ce/issues/55035

Post by Erik Faye-Lund
Perhaps a we can also somehow map emails to irc nicks?

We maintain such a list on the wiki already:
https://dri.freedesktop.org/wiki/WhosWho/

We could add this mapping to a file, but how would you use it? Are you
suggesting a bot would directly query people on irc for each MR matching
their REVIEWERS group?

I think direct email + mailing list + irc channel would be enough there,
direct irc query feels too spammy.

Daniel Stone

2018-12-07 20:24:08 UTC

Permalink

Hi,

Post by Eric Engestrom

Post by Erik Faye-Lund
Automated emails (and perhaps IRC bot) would be really nice.

Agreed. Email would be great to help with the transition.
There's work currently being done on GitLab to allow for mailing lists
to be notified; this should cover 'new MR' as well.
If we need this feature before GitLab is done, it should be possible to
write a bot using the webhooks, just needs someone to take the time to
do it :)
For IRC, there's already some integration, but it's limited to notifying
https://docs.gitlab.com/ee/user/project/integrations/irker.html
There's an open issue about adding more events, but it hasn't seen much
https://gitlab.com/gitlab-org/gitlab-ce/issues/7965

Wayland uses a couple of eventd plugins chained together:
https://github.com/sardemff7/git-eventc

That notifies the channel when issues and MRs are opened or closed and
on push as well, including things like the labels. It's been pretty
useful so far.

Post by Eric Engestrom

Post by Erik Faye-Lund
Even better if it could be hooked up to scripts/get_reviewer.pl, and
automatically CC "the right people".

Side note, I've been rewriting that script, although I need to send v2
https://patchwork.freedesktop.org/patch/226256/
I would be trivial to hook that into a bot we'd write, but I don't think
GitLab has support for something like this. I just opened an issue about
https://gitlab.com/gitlab-org/gitlab-ce/issues/55035

This already exists, as an EE-only feature called 'code owners':
https://docs.gitlab.com/ee/user/project/code_owners.html
https://gitlab.com/gitlab-org/gitlab-ee/issues/1012

Cheers,
Daniel

Eric Anholt

2018-12-06 20:03:08 UTC

Permalink

Reviewed-by: Eric Anholt <***@anholt.net>

Dylan Baker

2018-12-06 21:48:44 UTC

Permalink

Quoting Jordan Justen (2018-12-05 15:32:05)

Post by Jordan Justen
This documents a process for using GitLab Merge Requests as an second
way to submit code changes for Mesa. Only one of the two methods is
allowed for each patch series.
We will *not* require all patches to be emailed. Some code changes may
be reviewed and merged without any discussion on the mesa-dev email
list.
* No longer require email. Allow submitter to choose email or a
GitLab merge request.
* Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
Matt, Michel and Rob.
---
docs/submittingpatches.html | 76 ++++++++++++++++++++++++++++++++++---
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/docs/submittingpatches.html b/docs/submittingpatches.html
index 92d954a2d09..21175988d0b 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -21,7 +21,7 @@
<li><a href="#guidelines">Basic guidelines</a>
<li><a href="#formatting">Patch formatting</a>
<li><a href="#testing">Testing Patches</a>
-<li><a href="#mailing">Mailing Patches</a>
+<li><a href="#submit">Submitting Patches</a>
<li><a href="#reviewing">Reviewing Patches</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>
@@ -42,8 +42,10 @@ components.
<code>git bisect</code>.)
<li>Patches should be properly <a href="#formatting">formatted</a>.
<li>Patches should be sufficiently <a href="#testing">tested</a> before submitting.
-<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
-for <a href="#reviewing">review</a> using <code>git send-email</code>.
+<li>Patches should be <a href="#submit">submitted</a>
+to <a href="#mailing">mesa-dev</a> or with
+a <a href="#merge-request">merge request</a>
+for <a href="#reviewing">review</a>.
</ul>
@@ -180,10 +182,19 @@ run.
</p>
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
<p>
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your changes.
+</p>
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches. (Note that you may need to contact
your email administrator for this.)
</p>
+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.
+</p>
+<p>
+ <ul>
+ <li>Mesa changes affecting all drivers: mesa
+ <li>Hardware vendor specific code: amd, intel, nvidia, ...
+ <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
+ radv, vc4, ...
+ <li>Other tag examples: gallium, util
+ </ul>
+</p>
+<p>
+ If you revise your patches based on code review and push an update
+ to your branch, you should maintain a <strong>clean</strong> history
+ in your patches. There should not be "fixup" patches in the history.
+ The series should be buildable and functional after every commit
+ whenever you push the branch.
+</p>
+<p>
+ It is your responsibility to keep the MR alive and making progress,
+ as there are no guarantees that a Mesa dev will independently take
+ interest in it.
+</p>
+<p>
+ <ul>
+ <li>Make changes and update your branch based on feedback
+ <li>Old, stale MR may be closed, but you can reopen it if you
+ still want to pursue the changes
+ <li>You should periodically check to see if your MR needs to be
+ rebased
+ <li>Make sure your MR is closed if your patches get pushed outside
+ of GitLab
+ </ul>
+</p>
+
<h2 id="reviewing">Reviewing Patches</h2>
+<p>
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
+ Requests</a> page.
+</p>
+
<p>
When you've reviewed a patch on the mailing list, please be unambiguous
about your review. That is, state either
--
2.20.0.rc2
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Acked-by: Dylan Baker <***@pnwbakers.com>

Nicolai Hähnle

2018-12-06 21:57:09 UTC

Permalink

This link is broken.

What's the best way to get a feel for how the review process would work
in practice?

Cheers,
Nicolai

Post by Jordan Justen
+</p>
+
<p>
When you've reviewed a patch on the mailing list, please be unambiguous
about your review. That is, state either

--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.

Jason Ekstrand

2018-12-06 22:08:17 UTC

Permalink

Post by Jordan Justen

branch</a>

Post by Jordan Justen
@@ -42,8 +42,10 @@ components.
<code>git bisect</code>.)
<li>Patches should be properly <a href="#formatting">formatted</a>.
<li>Patches should be sufficiently <a href="#testing">tested</a>

before submitting.

Post by Jordan Justen
-<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
-for <a href="#reviewing">review</a> using <code>git send-email</code>.
+<li>Patches should be <a href="#submit">submitted</a>
+to <a href="#mailing">mesa-dev</a> or with
+a <a href="#merge-request">merge request</a>
+for <a href="#reviewing">review</a>.
</ul>
@@ -180,10 +182,19 @@ run.
</p>
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
<p>
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your changes.
+</p>
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches. (Note that

you may need to contact

Post by Jordan Justen
your email administrator for this.)
</p>
+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.
+</p>
+<p>
+ <ul>
+ <li>Mesa changes affecting all drivers: mesa
+ <li>Hardware vendor specific code: amd, intel, nvidia, ...
+ <li>Driver specific code: anvil, freedreno, i965, iris, radeonsi,
+ radv, vc4, ...
+ <li>Other tag examples: gallium, util
+ </ul>
+</p>
+<p>
+ If you revise your patches based on code review and push an update
+ to your branch, you should maintain a <strong>clean</strong> history
+ in your patches. There should not be "fixup" patches in the history.
+ The series should be buildable and functional after every commit
+ whenever you push the branch.
+</p>
+<p>
+ It is your responsibility to keep the MR alive and making progress,
+ as there are no guarantees that a Mesa dev will independently take
+ interest in it.
+</p>
+<p>
+ <ul>
+ <li>Make changes and update your branch based on feedback
+ <li>Old, stale MR may be closed, but you can reopen it if you
+ still want to pursue the changes
+ <li>You should periodically check to see if your MR needs to be
+ rebased
+ <li>Make sure your MR is closed if your patches get pushed outside
+ of GitLab
+ </ul>
+</p>
+
<h2 id="reviewing">Reviewing Patches</h2>
+<p>
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests

">Merge

Post by Jordan Justen
+ Requests</a> page.

This link is broken.
What's the best way to get a feel for how the review process would work
in practice?

Try it? That sounds a bit tongue-in-cheek but I really do think that's the
only way we're going to properly answer that question. I've been using
GitLab review for quite some time now in other contexts and it's worked out
way better there than I expected. I'm not sure exactly how it'll work out
for mesa but X and Wayland are both using it and they formerly had very
similar flows to mesa.

--Jason

Jordan Justen

2018-12-06 22:37:13 UTC

Permalink

Post by Nicolai HÃ¤hnle

Post by Jordan Justen
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
+ Requests</a> page.

This link is broken.

Yeah, we'll have to enable merge requests on the Mesa project before
it'll work. :)

I just used the crucible url and s/crucible/mesa/.

https://gitlab.freedesktop.org/mesa/crucible/merge_requests

-Jordan

Erik Faye-Lund

2018-12-07 09:21:25 UTC

Permalink

Post by Jordan Justen
This documents a process for using GitLab Merge Requests as an second
way to submit code changes for Mesa. Only one of the two methods is
allowed for each patch series.
We will *not* require all patches to be emailed. Some code changes may
be reviewed and merged without any discussion on the mesa-dev email
list.
* No longer require email. Allow submitter to choose email or a
GitLab merge request.
* Various feedback from Brian, Daniel, Dylan, Eric, Erik, Jason,
Matt, Michel and Rob.
---
docs/submittingpatches.html | 76 ++++++++++++++++++++++++++++++++++-
--
1 file changed, 71 insertions(+), 5 deletions(-)
diff --git a/docs/submittingpatches.html
b/docs/submittingpatches.html
index 92d954a2d09..21175988d0b 100644
--- a/docs/submittingpatches.html
+++ b/docs/submittingpatches.html
@@ -21,7 +21,7 @@
<li><a href="#guidelines">Basic guidelines</a>
<li><a href="#formatting">Patch formatting</a>
<li><a href="#testing">Testing Patches</a>
-<li><a href="#mailing">Mailing Patches</a>
+<li><a href="#submit">Submitting Patches</a>
<li><a href="#reviewing">Reviewing Patches</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>
@@ -42,8 +42,10 @@ components.
<code>git bisect</code>.)
<li>Patches should be properly <a href="#formatting">formatted</a>.
<li>Patches should be sufficiently <a href="#testing">tested</a> before submitting.
-<li>Patches should be submitted to <a href="#mailing">mesa-dev</a>
-for <a href="#reviewing">review</a> using <code>git send-
email</code>.
+<li>Patches should be <a href="#submit">submitted</a>
+to <a href="#mailing">mesa-dev</a> or with
+a <a href="#merge-request">merge request</a>
+for <a href="#reviewing">review</a>.
</ul>
@@ -180,10 +182,19 @@ run.
</p>
-<h2 id="mailing">Mailing Patches</h2>
+<h2 id="submit">Submitting Patches</h2>
<p>
+Patches may be submitted to the Mesa project by
+<a href="#mailing">email</a> or with a
+GitLab <a href="#merge-request">merge request</a>. To prevent
+duplicate code review, only use one method to submit your changes.
+</p>
+
+<h3 id="mailing">Mailing Patches</h3>
+
+<p>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
When submitting a patch make sure to use
@@ -217,8 +228,63 @@ disabled before sending your patches. (Note that
you may need to contact
your email administrator for this.)
</p>
+<h3 id="merge-request">GitLab Merge Requests</h3>
+
+<p>
+ <a href="https://gitlab.freedesktop.org/mesa/mesa">GitLab</a> Merge
+ Requests (MR) can also be used to submit patches for Mesa.
+</p>
+
+<p>
+ If the MR may have interest for most of the Mesa community, you can
+ send an email to the mesa-dev email list including a link to the MR.
+ Don't send the patch to mesa-dev, just the MR link.
+</p>
+<p>
+ <ul>
+ <li>Mesa changes affecting all drivers: mesa
+ <li>Hardware vendor specific code: amd, intel, nvidia, ...
+ <li>Driver specific code: anvil, freedreno, i965, iris,
radeonsi,
+ radv, vc4, ...
+ <li>Other tag examples: gallium, util
+ </ul>
+</p>
+<p>
+ If you revise your patches based on code review and push an update
+ to your branch, you should maintain a <strong>clean</strong> history
+ in your patches. There should not be "fixup" patches in the
history.
+ The series should be buildable and functional after every commit
+ whenever you push the branch.
+</p>
+<p>
+ It is your responsibility to keep the MR alive and making
progress,
+ as there are no guarantees that a Mesa dev will independently take
+ interest in it.
+</p>
+<p>
+ <ul>
+ <li>Make changes and update your branch based on feedback
+ <li>Old, stale MR may be closed, but you can reopen it if you
+ still want to pursue the changes
+ <li>You should periodically check to see if your MR needs to be
+ rebased
+ <li>Make sure your MR is closed if your patches get pushed outside
+ of GitLab
+ </ul>
+</p>
+
<h2 id="reviewing">Reviewing Patches</h2>
+<p>
+ To participate in code review, you should monitor the
+ <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">
+ mesa-dev</a> email list and the GitLab
+ Mesa <a href="
https://gitlab.freedesktop.org/mesa/mesa/merge_requests">Merge
+ Requests</a> page.
+</p>
+
<p>
When you've reviewed a patch on the mailing list, please be
unambiguous
about your review. That is, state either

Eric Engestrom

2018-12-07 16:49:31 UTC

Permalink