Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Am Freitag, den 14.12.2018, 12:38 +0100 schrieb Samuel Pitoiset: > > On 12/13/18 9:27 PM, Gert Wollny wrote: > > IMHO allowing MRs is a good thing, so > >Acked-by: Gert Wollny > > > > Allowing MRs isn't a bad thing. The main problem IMHO is that now we > have to look at both emails and MRs, and I think we are probably > going to miss interesting/important changes. Gitlab also supports sending a notification when a new MR is opened, I think that is part of the "Watch" notification level or one can set it specifically in "User Settings/Notifications" for each project in the custom settings. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 12/13/18 9:27 PM, Gert Wollny wrote: IMHO allowing MRs is a good thing, so Acked-by: Gert Wollny Allowing MRs isn't a bad thing. The main problem IMHO is that now we have to look at both emails and MRs, and I think we are probably going to miss interesting/important changes. I've added a little remark below. Best, Gert Am Mittwoch, den 05.12.2018, 15:32 -0800 schrieb 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. 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 --- 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 @@ Basic guidelines Patch formatting Testing Patches -Mailing Patches +Submitting Patches Reviewing Patches Nominating a commit for a stable branch Criteria for accepting patches to the stable branch @@ -42,8 +42,10 @@ components. git bisect.) Patches should be properly formatted. Patches should be sufficiently tested before submitting. -Patches should be submitted to mesa-dev -for review using git send- email. +Patches should be submitted +to mesa-dev or with +a merge request +for review. @@ -180,10 +182,19 @@ run. -Mailing Patches +Submitting Patches -Patches should be sent to the mesa-dev mailing list for review: +Patches may be submitted to the Mesa project by +email or with a +GitLab merge request. To prevent +duplicate code review, only use one method to submit your changes. + + +Mailing Patches + + +Patches may be sent to the mesa-dev mailing list for review: https://lists.freedesktop.org/mailman/listinfo/mesa-dev;>; mesa-dev@lists.freedesktop.org. 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.) +GitLab Merge Requests + + + https://gitlab.freedesktop.org/mesa/mesa;>GitLab; Merge + Requests (MR) can also be used to submit patches for Mesa. + + + + 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. + + + Add labels to your MR to help reviewers find it. For example: + +Mesa changes affecting all drivers: mesa +Hardware vendor specific code: amd, intel, nvidia, ... +Driver specific code: anvil, freedreno, i965, iris, radeonsi, + radv, vc4, ... +Other tag examples: gallium, util + + + + If you revise your patches based on code review and push an update + to your branch, you should maintain a clean 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. + + + 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. + + + Some other notes: + +Make changes and update your branch based on feedback +Old, stale MR may be closed, but you can reopen it if you + still want to pursue the changes +You should periodically check to see if your MR needs to be + rebased Just a remark: With virglrenderer I actually get a notification email when a MR can no longer be applied cleanly, I don't think I had to configure this explicitely. +Make sure your MR is closed if your patches get pushed outside + of GitLab + + + Reviewing Patches + + To participate in code review, you should monitor the + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> ; + mesa-dev email list and the GitLab + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_reque sts">Merge + Requests page. + + When you've reviewed a patch on the mailing list, please be unambiguous about your review. That is, state either ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, 2018-12-13 at 21:27 +0100, Gert Wollny wrote: > IMHO allowing MRs is a good thing, so > Acked-by: Gert Wollny > > I've added a little remark below. > > Best, > Gert > > Am Mittwoch, den 05.12.2018, 15:32 -0800 schrieb Jordan Justen: > > +You should periodically check to see if your MR needs to > > be > > + rebased > Just a remark: With virglrenderer I actually get a notification email > when a MR can no longer be applied cleanly, I don't think I had to > configure this explicitely. Oh, very good point. That's a pretty awesome feature :) ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, 2018-12-13 at 18:07 +0100, Axel Davy wrote: > On 13/12/2018 17:57, Mathias Fröhlich wrote: > > Hi, > > Initially it seemed to me that I am about the only one sticking > > with mailing lists. > > And I personally feel like a too small contributor to really try to > > influence your > > decisions too much. But these recent hand full of mails all tell me > > that I am not > > that alone. I personally did contribute to several projects during > > the past years. > > All that only in part time, thus it had to be *very* efficient for > > myself. And that is > > something that I achieved by a consistent 'interface' to all those > > projects. Just > > my widely used and highly convenient mail client. So, all that > > worked in a sufficiently > > efficient way because I could combine this kind of 'work' even with > > my private mail > > that I could handle in between with that single 'interface'. So > > going to any web site > > there is already a detour and having multiple of them for each such > > project gives an > > even longer detour. Okay today it's mostly mesa that is left as > > well as a communication > > middle end used in vizsim applications. But going away too far away > > from a mailing list > > will be mostly a loss of efficiency for me. > > As I said, my two cents, that should not keep you all from doing > > changes that finally > > increase your all efficiency ... > > > > best > > > > Mathias > > > > > > > > Hi, > > > I have to add my voice here as well. > > Even though I do not feel able to give review for most of the mesa > code > base, > I appreciate to have all patches in the mailing list in my mail > client. > > From time to time, I give feedback for some set of patches, for > example > when I see patches related to dri3 or that could impact Nine. > > It also enables me to get an overview of all the recent works and > new > features Nine could use. > > I feel like if most patches go through MR without getting a mail > feedback, I would not be able to do those as efficiently. > > I would appreciate if I could *flag* some files or directories, and > when > a MR impacts those (for example dri3 files, gallium interface, > gallium > Nine, etc), > I could get an automated mail with a summary of the MR, in order to > encourage me to look at it. > AFAIU, the proposal is to always send the cover-letter of the series to the mailing-list with a link to the PR. Shouldn't that show you the summary (with a list of changed files) anyway, so it should be as easy as it currently is to figure out what to review? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
IMHO allowing MRs is a good thing, so Acked-by: Gert Wollny I've added a little remark below. Best, Gert Am Mittwoch, den 05.12.2018, 15:32 -0800 schrieb 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. > > 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 > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable > branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested > before submitting. > -Patches should be submitted to mesa-dev > -for review using git send- > email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;>; > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab; > Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, > radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean > 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased Just a remark: With virglrenderer I actually get a notification email when a MR can no longer be applied cleanly, I don't think I had to configure this explicitely. > +Make sure your MR is closed if your patches get pushed > outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > ; > + mesa-dev email list and the GitLab > + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_reque > sts">Merge > + Requests page. > + > + > > When you've reviewed a patch on the mailing list, please be > unambiguous > about your review. That is, state either ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Jason Ekstrand writes: > On Thu, Dec 13, 2018 at 11:07 AM Axel Davy wrote: > >> On 13/12/2018 17:57, Mathias Fröhlich wrote: >> > Hi, >> > Initially it seemed to me that I am about the only one sticking with >> mailing lists. >> > And I personally feel like a too small contributor to really try to >> influence your >> > decisions too much. But these recent hand full of mails all tell me that >> I am not >> > that alone. I personally did contribute to several projects during the >> past years. >> > All that only in part time, thus it had to be *very* efficient for >> myself. And that is >> > something that I achieved by a consistent 'interface' to all those >> projects. Just >> > my widely used and highly convenient mail client. So, all that worked in >> a sufficiently >> > efficient way because I could combine this kind of 'work' even with my >> private mail >> > that I could handle in between with that single 'interface'. So going to >> any web site >> > there is already a detour and having multiple of them for each such >> project gives an >> > even longer detour. Okay today it's mostly mesa that is left as well as >> a communication >> > middle end used in vizsim applications. But going away too far away from >> a mailing list >> > will be mostly a loss of efficiency for me. >> > As I said, my two cents, that should not keep you all from doing changes >> that finally >> > increase your all efficiency ... >> > >> > best >> > >> > Mathias >> > >> > >> > >> >> Hi, >> >> >> I have to add my voice here as well. >> >> Even though I do not feel able to give review for most of the mesa code >> base, >> I appreciate to have all patches in the mailing list in my mail client. >> >> From time to time, I give feedback for some set of patches, for example >> when I see patches related to dri3 or that could impact Nine. >> >> It also enables me to get an overview of all the recent works and new >> features Nine could use. >> >> I feel like if most patches go through MR without getting a mail >> feedback, I would not be able to do those as efficiently. >> > > I find it interesting that multiple people have had this same sentiment. > For me, the exact opposite is true. I'm someone who is responsible for > tracking and reviewing dozens of series at a time often with many patches > each. My current review backlog is probably 200 patches or more. Having > them in a linear stream in my mail isn't at all what I want because once I > blow past the first page (50 e-mails) stuff easily gets lost. The ability > to look at them at the series level, filter by tags, etc. is a huge > advantage. I also really like the fact that GitLab pulls all the comments > for the series together which makes cross-patch discussions easier to track. Complete agreement here. I'm both a regular contributor to Mesa (I've been in Jason's position before 100s-of-patches review backlog), and an occasional contributor to other projects (xserver is currently "occasional" status, unfortunately). I've found MRs to be better at tracking outsanding review for me than email, and I even use the email client written by cworth to try to solve this problem for us. > You can subscribe to an individual label and it will e-mail you every time > a MR is posted and tagged with that label. Now, it will probably take a > bit before we get label discipline sufficient that you can rely on it but > the mechanism is there. I'm curious if we can do something with this to replace discipline with software: https://about.gitlab.com/2016/08/19/applying-gitlab-labels-automatically/ signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Quoting Alex Deucher (2018-12-13 07:52:04) > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. > > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? > I feel like maybe I'm not using it right or missing features that > make it more useful and less awkward. I feel like the interface makes > it harder than it needs to be to see the actual changes in MR to be > reviewed. All of the links click through to the source view rather > than the patch view. > > Alex My perspective is based on the two other projects I work on, meson and alot (which, incidentally, I started working on alot because doing patch review on mailing lists is such a pain) - Using MRs drives a *lot* of drive by, one time, contributors. They run into some bug, write a patch, send an MR, and it's that easy. - It's easy to see all of the issues I've opened, and MRs I've opened (and the closed vs still open ones) quickly and easily. This makes tracking my outstanding work much easier (and is something I struggle with greatly) - All review happens in the same view. As I read the code I immediately see what other reviewers have said, which saves getting the same review comment multiple times and prevents conflicting review - by tagging issues (ala, Fixes #1234) in the commit message then pushing the commit the issue is automatically closed. This is super nice both to keep the bug tracker tidy and to figure out why an issue was closed. - not dealing with git am - V2s are automatically updated, and you can see the changes easily between various versions, as well as when each version was sent - Doesn't require huge setup to become efficient. I have now set up a pretty complicated email setup with notmuch+alot+afew to get tagging, open things in vim, added countless macros to vim, and sunk countless hours into alot to be able to do patch review efficiently. With github and gitlab, I open firefox, log in, and start doing work. - tags make it easy to see that something does or doesn't affect an area of the mesa that I know/care about My perspective coming into mesa was the doing mailing list review was very daunting and scary. There are a lot of rules that are expected that common clients (gmail) don't respect (don't send HTML mail), use inline comments, snip long quotes when they're not relevant. MRs take a lot of that away. We don't have to train people to use git-send-email *and* help them get it all set up. Hopefully that helps, Dylan signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:07 AM Axel Davy wrote: > On 13/12/2018 17:57, Mathias Fröhlich wrote: > > Hi, > > Initially it seemed to me that I am about the only one sticking with > mailing lists. > > And I personally feel like a too small contributor to really try to > influence your > > decisions too much. But these recent hand full of mails all tell me that > I am not > > that alone. I personally did contribute to several projects during the > past years. > > All that only in part time, thus it had to be *very* efficient for > myself. And that is > > something that I achieved by a consistent 'interface' to all those > projects. Just > > my widely used and highly convenient mail client. So, all that worked in > a sufficiently > > efficient way because I could combine this kind of 'work' even with my > private mail > > that I could handle in between with that single 'interface'. So going to > any web site > > there is already a detour and having multiple of them for each such > project gives an > > even longer detour. Okay today it's mostly mesa that is left as well as > a communication > > middle end used in vizsim applications. But going away too far away from > a mailing list > > will be mostly a loss of efficiency for me. > > As I said, my two cents, that should not keep you all from doing changes > that finally > > increase your all efficiency ... > > > > best > > > > Mathias > > > > > > > > Hi, > > > I have to add my voice here as well. > > Even though I do not feel able to give review for most of the mesa code > base, > I appreciate to have all patches in the mailing list in my mail client. > > From time to time, I give feedback for some set of patches, for example > when I see patches related to dri3 or that could impact Nine. > > It also enables me to get an overview of all the recent works and new > features Nine could use. > > I feel like if most patches go through MR without getting a mail > feedback, I would not be able to do those as efficiently. > I find it interesting that multiple people have had this same sentiment. For me, the exact opposite is true. I'm someone who is responsible for tracking and reviewing dozens of series at a time often with many patches each. My current review backlog is probably 200 patches or more. Having them in a linear stream in my mail isn't at all what I want because once I blow past the first page (50 e-mails) stuff easily gets lost. The ability to look at them at the series level, filter by tags, etc. is a huge advantage. I also really like the fact that GitLab pulls all the comments for the series together which makes cross-patch discussions easier to track. > I would appreciate if I could *flag* some files or directories, and when > a MR impacts those (for example dri3 files, gallium interface, gallium > Nine, etc), > I could get an automated mail with a summary of the MR, in order to > encourage me to look at it. > You can at least sort-of. If you go here: https://gitlab.freedesktop.org/mesa/mesa/labels You can subscribe to an individual label and it will e-mail you every time a MR is posted and tagged with that label. Now, it will probably take a bit before we get label discipline sufficient that you can rely on it but the mechanism is there. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 4:52 PM Alex Deucher wrote: > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. > > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? > I feel like maybe I'm not using it right or missing features that > make it more useful and less awkward. I feel like the interface makes > it harder than it needs to be to see the actual changes in MR to be > reviewed. All of the links click through to the source view rather > than the patch view. FWIW, the killer feature for me (if it actually works out in practice) is that I can keep track of patches that still need attention. With me doing this in my free time mostly still and the mesa list being pretty high volume, as well the tendency of giving regular reviews by r-b, it is very hard for me to track what MRs I still need to review, what has been pushed and what has been abandoned. If a series is not nearly trivial and I review it in one evening I lose track easily. Furthermore, I think patchwork has pretty much shown that deriving this from email is not going to work. Doing this in a more integrated environment where we can actually get people to indicate the status is IMO the only way to work towards getting there. As far as the actual reviewing itself, sure I agree that it needs improvement. > > Alex > > > > > On 12/6/18 12:32 AM, Jordan Justen wrote: > > > 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 > > > --- > > > 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 @@ > > > Basic guidelines > > > Patch formatting > > > Testing Patches > > > -Mailing Patches > > > +Submitting Patches > > > Reviewing Patches > > > Nominating a commit for a stable branch > > > Criteria for accepting patches to the stable > > > branch > > > @@ -42,8 +42,10 @@ components. > > > git bisect.) > > > Patches should be properly formatted. > > > Patches should be sufficiently tested before > > > submitting. > > > -Patches should be submitted to mesa-dev > > > -for review using git send-email. > > > +Patches should be submitted > > > +to mesa-dev or with > > > +a merge request > > > +for review. > > > > > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > > > > > -Mailing Patches > > > +Submitting Patches > > > > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > > +Patches may be submitted to the Mesa project by > > > +email or with a > > > +GitLab merge request. To prevent > > > +duplicate code review, only use one method to submit your changes. > > > + > > > + > > > +Mailing Patches > > > + > > > + > > > +Patches may be sent to the mesa-dev mailing list for review: > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > > mesa-dev@lists.freedesktop.org. > > > 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.) > > > > > > > > > +GitLab Merge Requests > > > + > > > + > > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > > + Requests (MR) can also be used to submit patches for Mesa. > > > + > > > + > > > + > > > + If the MR may have interest for most of the Mesa community, you can > > > + send
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 13/12/2018 17:57, Mathias Fröhlich wrote: Hi, Initially it seemed to me that I am about the only one sticking with mailing lists. And I personally feel like a too small contributor to really try to influence your decisions too much. But these recent hand full of mails all tell me that I am not that alone. I personally did contribute to several projects during the past years. All that only in part time, thus it had to be *very* efficient for myself. And that is something that I achieved by a consistent 'interface' to all those projects. Just my widely used and highly convenient mail client. So, all that worked in a sufficiently efficient way because I could combine this kind of 'work' even with my private mail that I could handle in between with that single 'interface'. So going to any web site there is already a detour and having multiple of them for each such project gives an even longer detour. Okay today it's mostly mesa that is left as well as a communication middle end used in vizsim applications. But going away too far away from a mailing list will be mostly a loss of efficiency for me. As I said, my two cents, that should not keep you all from doing changes that finally increase your all efficiency ... best Mathias Hi, I have to add my voice here as well. Even though I do not feel able to give review for most of the mesa code base, I appreciate to have all patches in the mailing list in my mail client. From time to time, I give feedback for some set of patches, for example when I see patches related to dri3 or that could impact Nine. It also enables me to get an overview of all the recent works and new features Nine could use. I feel like if most patches go through MR without getting a mail feedback, I would not be able to do those as efficiently. I would appreciate if I could *flag* some files or directories, and when a MR impacts those (for example dri3 files, gallium interface, gallium Nine, etc), I could get an automated mail with a summary of the MR, in order to encourage me to look at it. Yours, Axel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:41 AM Jason Ekstrand wrote: > > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: >> >> On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: >> > >> > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset >> > wrote: >> > > >> > > Personally, I will continue to use the list, at least for a simplicity >> > > point of view. I'm not sure if using a new tool will improve quality and >> > > code review process. >> > > >> > > Though, if the majority reports that is really nice to use, I will >> > > probably change my mind. Not a strong reject. >> > >> > I agree. We've been using the MR interface for xf86-video-amdgpu and >> > I find it awkward compared to the mailing list. Maybe it just takes >> > getting used to. I also feel less inclined to do drive by patch >> > review if I have to explicitly delve into the browser to look at the >> > outstanding MRs. Over email, sometimes I see a patch set in my in box >> > that piques my interest and I find some time to review it when I might >> > not have otherwise if the bar were higher. >> >> FWIW I also do a lot of drive-by reviews. Perhaps those aren't >> valuable -- dunno. Either way, if it's not in email, I won't end up >> seeing it. > > > I find this commentary on drive-by reviews interesting. Personally, I don't > find going to the web interface to be a problem at all but that may be > because my e-mail is also in my web browser and it's as easy as clicking a > link. Also, since I'm an active contributor who's likely to be making MRs > and commenting on various MRs on a regular basis, I'm already on the MR page > looking at it. If you randomly scrolled through the MR list in the same way > you randomly scroll through the mailing list, would you be equally inclined > to give drive-by reviews? That's an honest hypothetical question. Less so, but greater than 0. My e-mail client is also web-based (gmail). I auto-archive all mailing list email, and attach various labels like "mesa-dev" and "dri-devel" (yeah, I'm creative). I will frequently run through those emails looking at subjects of new ones. I will also click into patches, and look at them for 0.5-3 seconds each (I've gotten good at this). If they're short, or I happen to notice something, or they're interesting, or they're in "my domain", I'll just reply. Otherwise I do nothing. With a web UI... I wouldn't end up on it in the first place. I always have an email client open. I don't have gitlab open. Even if I did, I'd have to keep flipping between various projects I might be interested in. I also like to stay apprised of various goings on, like the various display work going on in other drivers. Having it all stream through email is convenient. If it's hidden behind some web UI ... I'll never look at it. Here's a practical example, even if it's not a 100% match for this situation -- a company I formerly worked at had a vibrant mailing list of ex-employees. Lots of traffic, job postings, etc. Then a well-intentioned HR person at that company who managed the community created an online job board and additional environment, with additional logins, etc -- where posts, mailing lists, etc would go. The job board posts are gone now, the new mailing lists never got traffic since it no longer went to people's primary emails, and the community is now dead. They rolled it all back, but it never recovered. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 11:41 AM Jason Ekstrand wrote: > > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: >> >> On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: >> > >> > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset >> > wrote: >> > > >> > > Personally, I will continue to use the list, at least for a simplicity >> > > point of view. I'm not sure if using a new tool will improve quality and >> > > code review process. >> > > >> > > Though, if the majority reports that is really nice to use, I will >> > > probably change my mind. Not a strong reject. >> > >> > I agree. We've been using the MR interface for xf86-video-amdgpu and >> > I find it awkward compared to the mailing list. Maybe it just takes >> > getting used to. I also feel less inclined to do drive by patch >> > review if I have to explicitly delve into the browser to look at the >> > outstanding MRs. Over email, sometimes I see a patch set in my in box >> > that piques my interest and I find some time to review it when I might >> > not have otherwise if the bar were higher. >> >> FWIW I also do a lot of drive-by reviews. Perhaps those aren't >> valuable -- dunno. Either way, if it's not in email, I won't end up >> seeing it. > > > I find this commentary on drive-by reviews interesting. Personally, I don't > find going to the web interface to be a problem at all but that may be > because my e-mail is also in my web browser and it's as easy as clicking a > link. Also, since I'm an active contributor who's likely to be making MRs > and commenting on various MRs on a regular basis, I'm already on the MR page > looking at it. If you randomly scrolled through the MR list in the same way > you randomly scroll through the mailing list, would you be equally inclined > to give drive-by reviews? That's an honest hypothetical question. > I'm still very much email driven and most of the projects I work most in are still email based. I think the problem is that you don't see the patches, just the MR list so you have to kind of guess based on the MR names. When I'm reading through email, a function name or something in a patch title may catch my eye and I'll then take a look at that patch and probably the whole series. E.g., "oh, they are changing foo(), I just did some work there a few weeks ago, let me take a look..", whereas the MR title may be something like "Restructure to support new extension BAR." which I may not really be too familiar with. Alex ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Hi, On Thursday, 13 December 2018 17:40:49 CET Jason Ekstrand wrote: > On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: > > > On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher > > wrote: > > > > > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > > > wrote: > > > > > > > > Personally, I will continue to use the list, at least for a simplicity > > > > point of view. I'm not sure if using a new tool will improve quality > > and > > > > code review process. > > > > > > > > Though, if the majority reports that is really nice to use, I will > > > > probably change my mind. Not a strong reject. > > > > > > I agree. We've been using the MR interface for xf86-video-amdgpu and > > > I find it awkward compared to the mailing list. Maybe it just takes > > > getting used to. I also feel less inclined to do drive by patch > > > review if I have to explicitly delve into the browser to look at the > > > outstanding MRs. Over email, sometimes I see a patch set in my in box > > > that piques my interest and I find some time to review it when I might > > > not have otherwise if the bar were higher. > > > > FWIW I also do a lot of drive-by reviews. Perhaps those aren't > > valuable -- dunno. Either way, if it's not in email, I won't end up > > seeing it. > > > > I find this commentary on drive-by reviews interesting. Personally, I > don't find going to the web interface to be a problem at all but that may > be because my e-mail is also in my web browser and it's as easy as clicking > a link. Also, since I'm an active contributor who's likely to be making > MRs and commenting on various MRs on a regular basis, I'm already on the MR > page looking at it. If you randomly scrolled through the MR list in the > same way you randomly scroll through the mailing list, would you be equally > inclined to give drive-by reviews? That's an honest hypothetical question. Initially it seemed to me that I am about the only one sticking with mailing lists. And I personally feel like a too small contributor to really try to influence your decisions too much. But these recent hand full of mails all tell me that I am not that alone. I personally did contribute to several projects during the past years. All that only in part time, thus it had to be *very* efficient for myself. And that is something that I achieved by a consistent 'interface' to all those projects. Just my widely used and highly convenient mail client. So, all that worked in a sufficiently efficient way because I could combine this kind of 'work' even with my private mail that I could handle in between with that single 'interface'. So going to any web site there is already a detour and having multiple of them for each such project gives an even longer detour. Okay today it's mostly mesa that is left as well as a communication middle end used in vizsim applications. But going away too far away from a mailing list will be mostly a loss of efficiency for me. As I said, my two cents, that should not keep you all from doing changes that finally increase your all efficiency ... best Mathias ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 9:56 AM Ilia Mirkin wrote: > On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher > wrote: > > > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > > wrote: > > > > > > Personally, I will continue to use the list, at least for a simplicity > > > point of view. I'm not sure if using a new tool will improve quality > and > > > code review process. > > > > > > Though, if the majority reports that is really nice to use, I will > > > probably change my mind. Not a strong reject. > > > > I agree. We've been using the MR interface for xf86-video-amdgpu and > > I find it awkward compared to the mailing list. Maybe it just takes > > getting used to. I also feel less inclined to do drive by patch > > review if I have to explicitly delve into the browser to look at the > > outstanding MRs. Over email, sometimes I see a patch set in my in box > > that piques my interest and I find some time to review it when I might > > not have otherwise if the bar were higher. > > FWIW I also do a lot of drive-by reviews. Perhaps those aren't > valuable -- dunno. Either way, if it's not in email, I won't end up > seeing it. > I find this commentary on drive-by reviews interesting. Personally, I don't find going to the web interface to be a problem at all but that may be because my e-mail is also in my web browser and it's as easy as clicking a link. Also, since I'm an active contributor who's likely to be making MRs and commenting on various MRs on a regular basis, I'm already on the MR page looking at it. If you randomly scrolled through the MR list in the same way you randomly scroll through the mailing list, would you be equally inclined to give drive-by reviews? That's an honest hypothetical question. --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 2018-12-13 4:52 p.m., Alex Deucher wrote: > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: >> >> Personally, I will continue to use the list, at least for a simplicity >> point of view. I'm not sure if using a new tool will improve quality and >> code review process. >> >> Though, if the majority reports that is really nice to use, I will >> probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. Hopefully we can get notifications of new MRs to the mailing lists at some point, but in the meantime, this should be solvable by tuning your notification settings in GitLab. That might even allow you to fine-tune what you receive notifications for, compared to the all-or-nothing with e-mail based review. > Out of curiosity what do others like about the MR interface? How are > you using it? What advantages does it give you over the mailing list? One things that's nice about the MR interface is the integration with the CI pipeline. But for me, it's not so much about the MR interface per se, but about the integration between different parts. With e-mail based review, it's sometimes a bit awkward to link between patches for testing / review and the final changes in Git, e.g. in bug reports. All of these things can be linked together by MRs. This should be even better once we use GitLab issues instead of Bugzilla. > I feel like the interface makes it harder than it needs to be to see > the actual changes in MR to be reviewed. All of the links click > through to the source view rather than the patch view. There are certainly still rough edges around review of MRs with multiple commits. There have been some improvements based on feedback provided by freedesktop.org folks to GitLab developers, but there's still need for more. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 13, 2018 at 10:52 AM Alex Deucher wrote: > > On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset > wrote: > > > > Personally, I will continue to use the list, at least for a simplicity > > point of view. I'm not sure if using a new tool will improve quality and > > code review process. > > > > Though, if the majority reports that is really nice to use, I will > > probably change my mind. Not a strong reject. > > I agree. We've been using the MR interface for xf86-video-amdgpu and > I find it awkward compared to the mailing list. Maybe it just takes > getting used to. I also feel less inclined to do drive by patch > review if I have to explicitly delve into the browser to look at the > outstanding MRs. Over email, sometimes I see a patch set in my in box > that piques my interest and I find some time to review it when I might > not have otherwise if the bar were higher. FWIW I also do a lot of drive-by reviews. Perhaps those aren't valuable -- dunno. Either way, if it's not in email, I won't end up seeing it. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, Dec 12, 2018 at 3:42 AM Samuel Pitoiset wrote: > > Personally, I will continue to use the list, at least for a simplicity > point of view. I'm not sure if using a new tool will improve quality and > code review process. > > Though, if the majority reports that is really nice to use, I will > probably change my mind. Not a strong reject. I agree. We've been using the MR interface for xf86-video-amdgpu and I find it awkward compared to the mailing list. Maybe it just takes getting used to. I also feel less inclined to do drive by patch review if I have to explicitly delve into the browser to look at the outstanding MRs. Over email, sometimes I see a patch set in my in box that piques my interest and I find some time to review it when I might not have otherwise if the bar were higher. Out of curiosity what do others like about the MR interface? How are you using it? What advantages does it give you over the mailing list? I feel like maybe I'm not using it right or missing features that make it more useful and less awkward. I feel like the interface makes it harder than it needs to be to see the actual changes in MR to be reviewed. All of the links click through to the source view rather than the patch view. Alex > > On 12/6/18 12:32 AM, Jordan Justen wrote: > > 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 > > --- > > 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 @@ > > Basic guidelines > > Patch formatting > > Testing Patches > > -Mailing Patches > > +Submitting Patches > > Reviewing Patches > > Nominating a commit for a stable branch > > Criteria for accepting patches to the stable > > branch > > @@ -42,8 +42,10 @@ components. > > git bisect.) > > Patches should be properly formatted. > > Patches should be sufficiently tested before > > submitting. > > -Patches should be submitted to mesa-dev > > -for review using git send-email. > > +Patches should be submitted > > +to mesa-dev or with > > +a merge request > > +for review. > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > -Mailing Patches > > +Submitting Patches > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > +Patches may be submitted to the Mesa project by > > +email or with a > > +GitLab merge request. To prevent > > +duplicate code review, only use one method to submit your changes. > > + > > + > > +Mailing Patches > > + > > + > > +Patches may be sent to the mesa-dev mailing list for review: > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > mesa-dev@lists.freedesktop.org. > > 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.) > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > + Requests (MR) can also be used to submit patches for Mesa. > > + > > + > > + > > + 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. > > + > > + > > + Add labels to your MR to help reviewers find it. For example: > > + > > +Mesa changes affecting all drivers: mesa > > +Hardware vendor specific code: amd, intel, nvidia, ... > > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > > + radv, vc4, ... > > +Other tag examples: gallium, util > > + > > + > > + > > + If you revise your patches based on code review and push an update > > + to your branch, you should maintain a clean 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. > > + > > + > > + 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. > > + > > + > > + Some other notes: > > + > > +Make changes and update your branch based on feedback > >
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Tue, 2018-12-11 at 18:06 -0600, Jason Ekstrand wrote: > Ping? > > I see about 5 acks/reviews, 3 of which are from Intel which doesn't > exactly seem like overwhelming consensus. However, we also haven't > had any debate in a while. Oh, absolutely, I'm very much a supporter of tring this out! With or without any bots or anything :) Acked-by: Erik Faye-Lund > I know some people are somewhat skeptical as to how well it will work > but they won't be able to see until we actually start experimenting > with it which we can't do until we allow MRs. Personally, I say we > should just turn on the option in GitLab and people can start playing > with it and see how it goes. We can always decide later that it was > a terrible idea and move all active MRs back to the list. > > --Jason > > On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone > wrote: > > Hi, > > > > On Sat, 8 Dec 2018 at 05:15, Eric Engestrom < > > eric.engest...@intel.com> wrote: > > > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: > > > > 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 > > > > 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. > > > > > > 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 > > > > 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 > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Personally, I will continue to use the list, at least for a simplicity point of view. I'm not sure if using a new tool will improve quality and code review process. Though, if the majority reports that is really nice to use, I will probably change my mind. Not a strong reject. On 12/6/18 12:32 AM, Jordan Justen wrote: 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 --- 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 @@ Basic guidelines Patch formatting Testing Patches -Mailing Patches +Submitting Patches Reviewing Patches Nominating a commit for a stable branch Criteria for accepting patches to the stable branch @@ -42,8 +42,10 @@ components. git bisect.) Patches should be properly formatted. Patches should be sufficiently tested before submitting. -Patches should be submitted to mesa-dev -for review using git send-email. +Patches should be submitted +to mesa-dev or with +a merge request +for review. @@ -180,10 +182,19 @@ run. -Mailing Patches +Submitting Patches -Patches should be sent to the mesa-dev mailing list for review: +Patches may be submitted to the Mesa project by +email or with a +GitLab merge request. To prevent +duplicate code review, only use one method to submit your changes. + + +Mailing Patches + + +Patches may be sent to the mesa-dev mailing list for review: https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> mesa-dev@lists.freedesktop.org. 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.) +GitLab Merge Requests + + + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge + Requests (MR) can also be used to submit patches for Mesa. + + + + 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. + + + Add labels to your MR to help reviewers find it. For example: + +Mesa changes affecting all drivers: mesa +Hardware vendor specific code: amd, intel, nvidia, ... +Driver specific code: anvil, freedreno, i965, iris, radeonsi, + radv, vc4, ... +Other tag examples: gallium, util + + + + If you revise your patches based on code review and push an update + to your branch, you should maintain a clean 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. + + + 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. + + + Some other notes: + +Make changes and update your branch based on feedback +Old, stale MR may be closed, but you can reopen it if you + still want to pursue the changes +You should periodically check to see if your MR needs to be + rebased +Make sure your MR is closed if your patches get pushed outside + of GitLab + + + Reviewing Patches + + To participate in code review, you should monitor the + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> + mesa-dev email list and the GitLab + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge + Requests page. + + When you've reviewed a patch on the mailing list, please be unambiguous about your review. That is, state either ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Tue, Dec 11, 2018 at 8:15 PM Ian Romanick wrote: > It's fairly common for Mesa developers to have several patch series on > the mailing list at a time. I believe most people will author these as > a continuous stream with either implicit dependencies (i.e., commit > messages in the second series with shader-db results that are impacted > by the first series) or explicit dependencies (i.e., second series uses > interfaces added or changed by the first). When I do this, I still like > to send the series out a separate sets of patches that accomplish > separate, logical tasks. > > We can't be the only project where people work like this, so what is the > common practice in other projects? I've thought of several possible > solutions, but each seems fatally flawed. > > - A single, possibly giant, MR defeats the ability to have separate > logical work units. > > - Splitting into multiple MRs based from master may not be practical or > possible without including some patches in both series. > > - Waiting to send the second series out may increase the lag in the > review process or lead to the submitter forgetting to submit. :) > What I would do would be to create a series of MRs that are each based on the one before. It gets a little weird because they get longer and longer but if you have some commentary in the MR header saying that it's based on another one, it shouldn't be so bad. If someone is doing patch-by-patch review, they can easily enough just comment on the patches unique to that MR. Sadly, that suggestion won't work quite the way you want as I don't think GitLab has a concept of dependent MRs. Maybe that's another thing to add to the feature request list? > On 12/5/18 3:32 PM, Jordan Justen wrote: > > 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 > > --- > > 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 @@ > > Basic guidelines > > Patch formatting > > Testing Patches > > -Mailing Patches > > +Submitting Patches > > Reviewing Patches > > Nominating a commit for a stable branch > > Criteria for accepting patches to the stable > branch > > @@ -42,8 +42,10 @@ components. > > git bisect.) > > Patches should be properly formatted. > > Patches should be sufficiently tested before > submitting. > > -Patches should be submitted to mesa-dev > > -for review using git send-email. > > +Patches should be submitted > > +to mesa-dev or with > > +a merge request > > +for review. > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > -Mailing Patches > > +Submitting Patches > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > +Patches may be submitted to the Mesa project by > > +email or with a > > +GitLab merge request. To prevent > > +duplicate code review, only use one method to submit your changes. > > + > > + > > +Mailing Patches > > + > > + > > +Patches may be sent to the mesa-dev mailing list for review: > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > mesa-dev@lists.freedesktop.org. > > 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.) > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > + Requests (MR) can also be used to submit patches for Mesa. > > + > > + > > + > > + 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. > > + > > + > > + Add labels to your MR to help reviewers find it. For example: > > + > > +Mesa changes affecting all drivers: mesa > > +Hardware vendor specific code: amd, intel, nvidia, ... > > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > > + radv, vc4, ... > > +Other tag examples: gallium, util > > + > > + > > + > > + If you revise your patches based on code review and push an update > > + to your branch, you should maintain a clean history > > + in your patches. There should not be "fixup" patches in
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
It's fairly common for Mesa developers to have several patch series on the mailing list at a time. I believe most people will author these as a continuous stream with either implicit dependencies (i.e., commit messages in the second series with shader-db results that are impacted by the first series) or explicit dependencies (i.e., second series uses interfaces added or changed by the first). When I do this, I still like to send the series out a separate sets of patches that accomplish separate, logical tasks. We can't be the only project where people work like this, so what is the common practice in other projects? I've thought of several possible solutions, but each seems fatally flawed. - A single, possibly giant, MR defeats the ability to have separate logical work units. - Splitting into multiple MRs based from master may not be practical or possible without including some patches in both series. - Waiting to send the second series out may increase the lag in the review process or lead to the submitter forgetting to submit. :) On 12/5/18 3:32 PM, Jordan Justen wrote: > 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 > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested before > submitting. > -Patches should be submitted to mesa-dev > -for review using git send-email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased > +Make sure your MR is closed if your patches get pushed outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > +
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Tue, Dec 11, 2018 at 7:06 PM Jason Ekstrand wrote: > > Ping? > > I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly > seem like overwhelming consensus. However, we also haven't had any debate in > a while. > > I know some people are somewhat skeptical as to how well it will work but > they won't be able to see until we actually start experimenting with it which > we can't do until we allow MRs. Personally, I say we should just turn on the > option in GitLab and people can start playing with it and see how it goes. > We can always decide later that it was a terrible idea and move all active > MRs back to the list. > I think some way to get cover-letters or similar (pref w/ link to mr) to list would be nice, but I'm defn ok w/ 'lets try it and then fine tune' approach. I think worst case is I overlook mr notification spam and someone has to poke me on irc, which is more or less the current worst case... Acked-by: Rob Clark > --Jason > > On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone wrote: >> >> Hi, >> >> On Sat, 8 Dec 2018 at 05:15, Eric Engestrom wrote: >> > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: >> > > 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 >> >> 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. >> >> > > 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 >> >> 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 >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, Dec 12, 2018 at 1:06 AM Jason Ekstrand wrote: > > Ping? > > I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly > seem like overwhelming consensus. However, we also haven't had any debate in > a while. FWIW, I'm fine with it too. Acked-by: Bas Nieuwenhuizen > > I know some people are somewhat skeptical as to how well it will work but > they won't be able to see until we actually start experimenting with it which > we can't do until we allow MRs. Personally, I say we should just turn on the > option in GitLab and people can start playing with it and see how it goes. > We can always decide later that it was a terrible idea and move all active > MRs back to the list. > > --Jason > > On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone wrote: >> >> Hi, >> >> On Sat, 8 Dec 2018 at 05:15, Eric Engestrom wrote: >> > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: >> > > 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 >> >> 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. >> >> > > 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 >> >> 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 >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Ping? I see about 5 acks/reviews, 3 of which are from Intel which doesn't exactly seem like overwhelming consensus. However, we also haven't had any debate in a while. I know some people are somewhat skeptical as to how well it will work but they won't be able to see until we actually start experimenting with it which we can't do until we allow MRs. Personally, I say we should just turn on the option in GitLab and people can start playing with it and see how it goes. We can always decide later that it was a terrible idea and move all active MRs back to the list. --Jason On Fri, Dec 7, 2018 at 2:24 PM Daniel Stone wrote: > Hi, > > On Sat, 8 Dec 2018 at 05:15, Eric Engestrom > wrote: > > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: > > > 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 > > 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. > > > > 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 > > 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 > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Hi, On Sat, 8 Dec 2018 at 05:15, Eric Engestrom wrote: > On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: > > 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 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. > > 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 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Friday, 2018-12-07 10:19:23 +0100, Erik Faye-Lund wrote: > On Wed, 2018-12-05 at 21:46 -0600, Jason Ekstrand wrote: > > On Wed, Dec 5, 2018 at 7:05 PM Jordan Justen < > > jordan.l.jus...@intel.com> wrote: > > > On 2018-12-05 15:44:18, Jason Ekstrand wrote: > > > > On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen < > > > jordan.l.jus...@intel.com> > > > > wrote: > > > > > -Mailing Patches > > > > > +Submitting Patches > > > > > > > > > > > > > > > -Patches should be sent to the mesa-dev mailing list for > > > review: > > > > > +Patches may be submitted to the Mesa project by > > > > > +email or with a > > > > > +GitLab merge request. To prevent > > > > > +duplicate code review, only use one method to submit your > > > changes. > > > > > + > > > > > > > > > > > > > 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. > > > > > > > > > + > > > > > +Mailing Patches > > > > > + > > > > > + > > > > > +Patches may be sent to the mesa-dev mailing list for review: > > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > > > > mesa-dev@lists.freedesktop.org. > > > > > 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.) > > > > > > > > > > > > > > > +GitLab Merge Requests > > > > > + > > > > > + > > > > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab > > > Merge > > > > > + Requests (MR) can also be used to submit patches for Mesa. > > > > > + > > > > > + > > > > > + > > > > > + 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. > > > > > 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. > > 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 > 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 > 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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wednesday, 2018-12-05 15:32:05 -0800, Jordan Justen wrote: > 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 Reviewed-by: Eric Engestrom > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested before > submitting. > -Patches should be submitted to mesa-dev > -for review using git send-email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased > +Make sure your MR is closed if your patches get pushed outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > + mesa-dev email list and the GitLab > + Mesa href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge > + Requests page. > + > + > > 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 > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Reviewed-by: Erik Faye-Lund On Wed, 2018-12-05 at 15:32 -0800, Jordan Justen wrote: > 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 > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable > branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested > before submitting. > -Patches should be submitted to mesa-dev > -for review using git send- > email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab > Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, > radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean > 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased > +Make sure your MR is closed if your patches get pushed > outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > + mesa-dev email list and the GitLab > + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge > + Requests page. > + > + > > When you've reviewed a patch on the mailing list, please be > unambiguous > about your review. That is, state either ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, 2018-12-05 at 21:46 -0600, Jason Ekstrand wrote: > On Wed, Dec 5, 2018 at 7:05 PM Jordan Justen < > jordan.l.jus...@intel.com> wrote: > > On 2018-12-05 15:44:18, Jason Ekstrand wrote: > > > On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen < > > jordan.l.jus...@intel.com> > > > wrote: > > > > -Mailing Patches > > > > +Submitting Patches > > > > > > > > > > > > -Patches should be sent to the mesa-dev mailing list for > > review: > > > > +Patches may be submitted to the Mesa project by > > > > +email or with a > > > > +GitLab merge request. To prevent > > > > +duplicate code review, only use one method to submit your > > changes. > > > > + > > > > > > > > > > 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. > > > > > > > + > > > > +Mailing Patches > > > > + > > > > + > > > > +Patches may be sent to the mesa-dev mailing list for review: > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > > > mesa-dev@lists.freedesktop.org. > > > > 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.) > > > > > > > > > > > > +GitLab Merge Requests > > > > + > > > > + > > > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab > > Merge > > > > + Requests (MR) can also be used to submit patches for Mesa. > > > > + > > > > + > > > > + > > > > + 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. > > > 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. 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? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 2018-12-06 13:57:09, Nicolai Hähnle wrote: > On 06.12.18 00:32, Jordan Justen wrote: > > + To participate in code review, you should monitor the > > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > + mesa-dev email list and the GitLab > > + Mesa > href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge > > + Requests 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Thu, Dec 6, 2018 at 3:57 PM Nicolai Hähnle wrote: > On 06.12.18 00:32, Jordan Justen wrote: > > 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 > > --- > > 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 @@ > > Basic guidelines > > Patch formatting > > Testing Patches > > -Mailing Patches > > +Submitting Patches > > Reviewing Patches > > Nominating a commit for a stable branch > > Criteria for accepting patches to the stable > branch > > @@ -42,8 +42,10 @@ components. > > git bisect.) > > Patches should be properly formatted. > > Patches should be sufficiently tested > before submitting. > > -Patches should be submitted to mesa-dev > > -for review using git send-email. > > +Patches should be submitted > > +to mesa-dev or with > > +a merge request > > +for review. > > > > > > > > @@ -180,10 +182,19 @@ run. > > > > > > > > -Mailing Patches > > +Submitting Patches > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > +Patches may be submitted to the Mesa project by > > +email or with a > > +GitLab merge request. To prevent > > +duplicate code review, only use one method to submit your changes. > > + > > + > > +Mailing Patches > > + > > + > > +Patches may be sent to the mesa-dev mailing list for review: > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > mesa-dev@lists.freedesktop.org. > > 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.) > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > + Requests (MR) can also be used to submit patches for Mesa. > > + > > + > > + > > + 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. > > + > > + > > + Add labels to your MR to help reviewers find it. For example: > > + > > +Mesa changes affecting all drivers: mesa > > +Hardware vendor specific code: amd, intel, nvidia, ... > > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > > + radv, vc4, ... > > +Other tag examples: gallium, util > > + > > + > > + > > + If you revise your patches based on code review and push an update > > + to your branch, you should maintain a clean 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. > > + > > + > > + 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. > > + > > + > > + Some other notes: > > + > > +Make changes and update your branch based on feedback > > +Old, stale MR may be closed, but you can reopen it if you > > + still want to pursue the changes > > +You should periodically check to see if your MR needs to be > > + rebased > > +Make sure your MR is closed if your patches get pushed outside > > + of GitLab > > + > > + > > + > > Reviewing Patches > > > > + > > + To participate in code review, you should monitor the > > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > + mesa-dev email list and the GitLab > > + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_requests > ">Merge > > + Requests 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 ___ mesa-dev mailing list
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 06.12.18 00:32, Jordan Justen wrote: 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 --- 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 @@ Basic guidelines Patch formatting Testing Patches -Mailing Patches +Submitting Patches Reviewing Patches Nominating a commit for a stable branch Criteria for accepting patches to the stable branch @@ -42,8 +42,10 @@ components. git bisect.) Patches should be properly formatted. Patches should be sufficiently tested before submitting. -Patches should be submitted to mesa-dev -for review using git send-email. +Patches should be submitted +to mesa-dev or with +a merge request +for review. @@ -180,10 +182,19 @@ run. -Mailing Patches +Submitting Patches -Patches should be sent to the mesa-dev mailing list for review: +Patches may be submitted to the Mesa project by +email or with a +GitLab merge request. To prevent +duplicate code review, only use one method to submit your changes. + + +Mailing Patches + + +Patches may be sent to the mesa-dev mailing list for review: https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> mesa-dev@lists.freedesktop.org. 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.) +GitLab Merge Requests + + + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge + Requests (MR) can also be used to submit patches for Mesa. + + + + 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. + + + Add labels to your MR to help reviewers find it. For example: + +Mesa changes affecting all drivers: mesa +Hardware vendor specific code: amd, intel, nvidia, ... +Driver specific code: anvil, freedreno, i965, iris, radeonsi, + radv, vc4, ... +Other tag examples: gallium, util + + + + If you revise your patches based on code review and push an update + to your branch, you should maintain a clean 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. + + + 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. + + + Some other notes: + +Make changes and update your branch based on feedback +Old, stale MR may be closed, but you can reopen it if you + still want to pursue the changes +You should periodically check to see if your MR needs to be + rebased +Make sure your MR is closed if your patches get pushed outside + of GitLab + + + Reviewing Patches + + To participate in code review, you should monitor the + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> + mesa-dev email list and the GitLab + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge + Requests page. This link is broken. What's the best way to get a feel for how the review process would work in practice? Cheers, Nicolai + + 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. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Quoting Jordan Justen (2018-12-05 15:32:05) > 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 > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested before > submitting. > -Patches should be submitted to mesa-dev > -for review using git send-email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased > +Make sure your MR is closed if your patches get pushed outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > + mesa-dev email list and the GitLab > + Mesa href="https://gitlab.freedesktop.org/mesa/mesa/merge_requests;>Merge > + Requests page. > + > + > > 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 > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev Acked-by: Dylan Baker signature.asc Description: signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
Jordan Justen writes: > 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. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, Dec 5, 2018 at 7:05 PM Jordan Justen wrote: > On 2018-12-05 15:44:18, Jason Ekstrand wrote: > > On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen > > wrote: > > > -Mailing Patches > > > +Submitting Patches > > > > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > > +Patches may be submitted to the Mesa project by > > > +email or with a > > > +GitLab merge request. To prevent > > > +duplicate code review, only use one method to submit your changes. > > > + > > > > > > > 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. > > > > > + > > > +Mailing Patches > > > + > > > + > > > +Patches may be sent to the mesa-dev mailing list for review: > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > > mesa-dev@lists.freedesktop.org. > > > 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.) > > > > > > > > > +GitLab Merge Requests > > > + > > > + > > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > > + Requests (MR) can also be used to submit patches for Mesa. > > > + > > > + > > > + > > > + 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. > 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 --Jason ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On 2018-12-05 15:44:18, Jason Ekstrand wrote: > On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen > wrote: > > -Mailing Patches > > +Submitting Patches > > > > > > -Patches should be sent to the mesa-dev mailing list for review: > > +Patches may be submitted to the Mesa project by > > +email or with a > > +GitLab merge request. To prevent > > +duplicate code review, only use one method to submit your changes. > > + > > > > 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. > > > + > > +Mailing Patches > > + > > + > > +Patches may be sent to the mesa-dev mailing list for review: > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > > mesa-dev@lists.freedesktop.org. > > 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.) > > > > > > +GitLab Merge Requests > > + > > + > > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > > + Requests (MR) can also be used to submit patches for Mesa. > > + > > + > > + > > + 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? 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] docs: Document GitLab merge request process (email alternative)
On Wed, Dec 5, 2018 at 5:32 PM Jordan Justen wrote: > 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 > --- > 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 @@ > Basic guidelines > Patch formatting > Testing Patches > -Mailing Patches > +Submitting Patches > Reviewing Patches > Nominating a commit for a stable branch > Criteria for accepting patches to the stable > branch > @@ -42,8 +42,10 @@ components. > git bisect.) > Patches should be properly formatted. > Patches should be sufficiently tested before > submitting. > -Patches should be submitted to mesa-dev > -for review using git send-email. > +Patches should be submitted > +to mesa-dev or with > +a merge request > +for review. > > > > @@ -180,10 +182,19 @@ run. > > > > -Mailing Patches > +Submitting Patches > > > -Patches should be sent to the mesa-dev mailing list for review: > +Patches may be submitted to the Mesa project by > +email or with a > +GitLab merge request. To prevent > +duplicate code review, only use one method to submit your changes. > + > 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. > + > +Mailing Patches > + > + > +Patches may be sent to the mesa-dev mailing list for review: > https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > mesa-dev@lists.freedesktop.org. > 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.) > > > +GitLab Merge Requests > + > + > + https://gitlab.freedesktop.org/mesa/mesa;>GitLab Merge > + Requests (MR) can also be used to submit patches for Mesa. > + > + > + > + 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. > + > + > + Add labels to your MR to help reviewers find it. For example: > + > +Mesa changes affecting all drivers: mesa > +Hardware vendor specific code: amd, intel, nvidia, ... > +Driver specific code: anvil, freedreno, i965, iris, radeonsi, > + radv, vc4, ... > +Other tag examples: gallium, util > + > + > + > + If you revise your patches based on code review and push an update > + to your branch, you should maintain a clean 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. > + > + > + 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. > + > + > + Some other notes: > + > +Make changes and update your branch based on feedback > +Old, stale MR may be closed, but you can reopen it if you > + still want to pursue the changes > +You should periodically check to see if your MR needs to be > + rebased > +Make sure your MR is closed if your patches get pushed outside > + of GitLab > + > + > + > Reviewing Patches > > + > + To participate in code review, you should monitor the > + https://lists.freedesktop.org/mailman/listinfo/mesa-dev;> > + mesa-dev email list and the GitLab > + Mesa https://gitlab.freedesktop.org/mesa/mesa/merge_requests > ">Merge > + Requests page. > + > + > > 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 > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
