This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Setup non-pushing gerrit instance for glibc.


On 11/12/19 1:53 PM, Gabriel F. T. Gomes wrote:
> Hi, community,
> 
> I finally used gerrit today.  Afterwards, I re-read this thread and here
> are my impressions and questions...
> 
> First of all, thanks for doing this.  :)
> 
> (CC'ing Sergio, because I don't know if he follows the list and I have a
> question for him)
> 
> 1. On Reviewed-by statements,
> 
> On Tue, 12 Nov 2019, Florian Weimer (Code Review) wrote:
>>
>> (I'm not sure if we can keep up the practice of adding Reviewed-By: lines
>> with Gerrit, because doing so creates a new patch version that needs
>> review.)
> 
> Florian noticed [1] that Reviewed-by statements might break our use of
> gerrit, because changing the commit message creates a new version of the
> patch.  Carlos mentioned that having the statements is useful for his
> metrics (I also like these fields, even though I don't use them as
> professionally).
> 
> If we are going to make gerrit push automatically one day, could we make
> it add the Reviewed-by statement automatically (based on the Reviewers
> field) when it pushes the change?  Maybe it doesn't even need to create a
> new version of the patch with the new commit message (although it would be
> nice if it could create some sort of link between the patch that actually
> gets commit and the patch displayed in the web interface (the same that is
> sent to the mailing list)).
> 
> If that is possible, then the patch author (or even the reviewers) won't
> need to change the commit message *during patch review*, which will avoid
> the creation of the new patch version.
> 
> [1] https://sourceware.org/ml/libc-alpha/2019-11/msg00404.html

I try to explain the consequences and tradeoffs here:

https://www.sourceware.org/ml/libc-alpha/2019-11/msg00439.html

Yes, gerrit can automatically add Reviewed-by.

Yes it will work if we stick to one workflow for patch submission.


> 3. On quote selection freedom,
> 
> When writing reviews by email, I like to select the precise amount of
> quotes (stitching different quotes together, and snipping unimportant
> parts) that I judge relevant for the conversation.  This is, obviously,
> a subjective judgment and I don't expect any tool to be able to do the
> same ever (famous last words?).  My point here is, I didn't find a way to
> do that (adjust the amount of quote and snipping) on the web interface.

Highlight with your cursor the text you want, and hit 'c' to comment.

> On Fri, 08 Nov 2019, Simon Marchi wrote:
>>
>> So please try to use "Quote" instead of "Reply" and quote
>> the relevant portion of what you are replying to (just like you would by
>> email), so that it appears in the notification email.
> 
> Simon mentioned [2] a "Quote" (button?).  Maybe that is what I wanted and
> couldn't find.

Highlight, and hit 'c'?

> Like others in the community, I think I prefer using email, which gives me
> freedom to select the amount of quoting and snipping I want, so maybe this
> will never be a problem to anyone.

Does highlighting and pressing 'c' resolve this for you?

> [2] https://sourceware.org/ml/libc-alpha/2019-11/msg00306.html
> 
> 4. On the "patches go missing" problem,
> 
> On Sun, 27 Oct 2019, Sergio Durigan Junior wrote:
>>
>> When the GDB community decided to give gerrit a try, the main problem it
>> was looking to solve was the "patches go missing" issue.  Basically, we
>> have a lot of patches being sent to the project and not so many
>> reviewers, so it's not uncommon for a patch to get "buried" waiting for
>> a review, and if the author doesn't ping it, it gets lost.  With gerrit,
>> you can get a pretty good view of all the patches that were submitted so
>> far, and you can always check if a patch is too old or if it's been too
>> long since the last interaction on it.
> 
> Can you (GDB community) already tell if gerrit actually helped with those
> patches that went missing, or is it to soon?  While I understand that
> gerrit has the potential to help with that, I also get the impression that
> patches will still pile up, but on the web interface, instead of on the
> mailing list.  I get this impression because our patchwork instance has
> piles of patches (btw, this is not an inquisitorial kind of question - my
> heart is still open for the new tool - I just want to know if you think
> gerrit is helping).

The problem with Patchwork is that it never helped with patch review.

Patchwork was simply another way to view the mailing list.

Gerrit today lets me actually complete reviews from start-to-finish and
can act as an assitance to me.

For example Gerrit already tells me if patches have merge conflicts, so
I can ignore those or tell people to rebase.

Eventually if we get some ci/cd tied to gerrit it should be really really
helpful for reviewers.

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]