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 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. Out of these, #1 and #2 are my biggest nusances with it, really. The PR/MR workflow tends to be all about the end-result, and not so much about the intermediate steps or the "story" they tell. And the UIs seems to reward such development rather than "the story matters"- development. I'm a bit worried that we'd start gradually changing how we developed; I saw this happen to some degree in my past experience. But at the same time, I do think that the benefits greatly outweigh the disadvantages. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
