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