<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, Nov 29, 2018 at 3:11 AM Erik Faye-Lund <<a href="mailto:erik.faye-lund@collabora.com">erik.faye-lund@collabora.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, 2018-11-27 at 17:13 -0800, Jordan Justen wrote:<br>
> This documents a mechanism for using GitLab Merge Requests as an<br>
> optional, secondary way to get code reviews for patchsets.<br>
> <br>
> We still require all patches to be emailed.<br>
> <br>
> Aside from the potential usage for code review comments, it might<br>
> also<br>
> help reviewers to find unmerged patchsets which need review.<br>
> (Assuming<br>
> it doesn't fall into the same fate of patchwork with hundreds of open<br>
> MRs.)<br>
> <br>
> Signed-off-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>><br>
> Cc: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>><br>
<br>
In lack of a better thread to discuss this, I'm going to hijack this<br>
one slightly. I hope I'm not too disturbing.<br>
<br>
For the better part of the last 4 years, I've been working on a large-<br>
ish (commercial, but open source) project that used GitHub PRs for<br>
contributions. It's a great way of working, for all the reasons we've<br>
already been through:<br>
- Structured reviews<br>
- Easy to get an overview what's happening<br>
- Prevent most bad merges due to CI<br>
<br>
But there's a few down-sides as well, compared to mailing list<br>
discussions, and these are the reasons I wanted to speak up. I'm a<br>
strong supporter for MRs, by the way. I just want these issues talked<br>
about. Also note that my experience here comes from GitHub, and not<br>
specifically from GitLab, but I suspect at least some of them apply<br>
here as well:<br>
<br>
1. There's usually no reasonable way of reviewing commit messages. You<br>
can add general comments on the development history, but it gets<br>
awkward to navigate between tabs to copy something to quote and stuff.<br>
In e-mail based reviews, you just click reply.<br>
<br>
2. It's often difficult to see comments on intermediate commits. At<br>
least on GitHub, intermediate changes gets collapsed as "out-of-date",<br>
because GitHub doesn't seem to care about clean development history.<br>
This tends to <br></blockquote><div><br></div><div>Both of the above are issues that various <a href="http://freedesktop.org">freedesktop.org</a> projects have raised with the GitLab community and are things they are actively improving.  It's not amazing yet, but there is active improvement for no-merge fast-forward commit series review.  In particular, I believe 2 is more-or-less sorted now.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
3. The MR list becomes something that needs to be maintained. Sometimes<br>
there's long-lived MRs that takes months to get merged, or just simply<br>
stall... and they all start piling up. That's fine in a mailing list,<br>
but it quickly gets cumbersome in a MR-based work-flow, as old MRs take<br>
up space in UIs, and makes it harder to see what's going on. So at some<br>
point, stale MRs should be closed IMO. This is going to need someone to<br>
either do it, or to maintain some bot to do it. And some developers<br>
tends to get angry when you close a MR after three months of inactivity<br>
just because nobody wanted to review it.<br></blockquote><div><br></div><div>I'm not too concerned with this.  We can easily write a bot or something.</div><div><br></div><div>--Jason<br></div></div></div>