Re: Proposal: Add review tags to patch review workflow.

[Luis Machado <luis.machado@arm.com>]

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!
>>
>