Re: Proposal: Add review tags to patch review workflow.
Luis Machado via Gdb <[email protected]>
| Newsgroups | gmane.comp.gdb.devel |
|---|---|
| Message-ID | <[email protected]> |
On 9/27/22 09:39, Luis Machado via Gdb wrote: > Hi Joel, > > Thanks for bringing this up. > > On 9/26/22 17:42, Joel Brobecker via Gdb wrote: >> Just thinking out loud... >> >>> I completely agree with the proposal. I really like the fact that it >>> makes communication less ambiguous. Following some process (or changing >>> the process) can feel a bit heavy for long-timers, but I think it makes >>> things much clearer for newcomers. >> >> Speaking of ambiguous, one thing that we used to do well in the past >> but then kind of got worse was the subject prefix we used to use >> to indicate the status of a patch. In particular, we used to reserve >> certain keywords for that in the subject (e.g. "RFA" vs "PATCH", or >> "OB" for obvious, etc). We lost that part, not sure exactly when, >> but I suspect sometime when we transitionned to Git. >> >> Something else also that I have been feeling the last year or two >> is that I'm not sure people now explicitly confirm to the list >> when a patch is pushed. > > I think that's been happening, yes. But the IRC bot mentions commits explicitly, and > developers tend to see updates in the git repo when they update the sources. > > With that said, in general the frequent GDB contributors tend to be quite busy (with > GDB or other things), so I'm inclined to say it is positive to have less steps to > take care of to push a change. > > For example, ChangeLog's were a big time sink, and we managed to get rid of that rule. I think > that was very positive. > > We still have other potential improvements waiting to be discussed, like auto-formatting of code > with some tool like clang-format. Time spent correcting formatting is not very useful. > >> >> The reason I mention this is to show that perhaps we're getting back >> to the fact that our email reviewing system is still email-based. >> One way to address the various limitations is by adding more >> processes, as suggested here. This has the good property of being >> fairly cheap to discuss and implement, at the cost of a small >> added overhead. I don't have a strong opinion about it, either >> for or against (and given the amount of time I have to contribute >> anyway, I don't think I should have a say). >> >> With that said, I have a feeling that switching to a system designed >> to manage patch submissions and reviews, no matter imperfect, is going >> to solve a lot of the limitations of the current email-based system. >> So that's another option worth reviewing from time to time, I think. >> I understand that selecting, deploying and trying new review systems >> requires a fair amount of effort. But having seen the benefits of >> using several different such systems, I am convinced that the gains >> will be very much worth whatever the drawbacks of that system might be. > > That's a fairly good point, and I agree. We tried a patch reviewing system (gerrit) > before. For me, at the time, it was obvious that the number of reviews increased > significantly. It was just easier to do reviews that way. If you had 5 minutes, you > could scan for a small change and give some feedback. The list of patches to-be-reviewed > was never forgotten. > > But back then we didn't want to risk alienating global maintainers that didn't like gerrit or > liked the e-mail system better, so we dropped that effort and put nothing back in its place. It feels > to me patch reviewing by non-contributors lost some of the traction it had gained with gerrit. Oops. I mean non-contributors -> non-maintainers. > > It is not a secret that some of us contributors would like to see improvements in this area, hence > my suggestion to address patch reviewing/more maintainers/CI-based testing as topics for the GDB BoF. > > But at the end of the day, it's up to the global maintainers to make a decision on this topic, or to > let contributors know they are open to adopting improvements. > > So, in summary, I see the proposal to add tags as a way to improve a patch reviewing system that > is not being capable of keeping up with demand. I doubt we would need such tagging if we had a > proper reviewing system in place (be it gerrit, patchworks or any other). > >> >>> Assuming we will go through with this proposal, it will need to be >>> documented on the wiki so we can easily refer people to the procedure. >>> Probably the ContributionChecklist page? >>> >>> https://sourceware.org/gdb/wiki/ContributionChecklist >>> >>> Will you be able to take care of this when needed (do you have write >>> access to the wiki)? >>> >>> In the mean time, message to others: please let us know if you agree >>> with this, it's difficult to know we have the support of the community >>> if everybody silently agrees! >> >