[petsc-dev] have requests for MR review indicate time expected to complete

[Satish Balay]

On Wed, 25 May 2022, Barry Smith wrote:

> 
> 
> > On May 25, 2022, at 12:06 PM, Satish Balay <balay at mcs.anl.gov> wrote:
> > 
> > On Wed, 25 May 2022, Matthew Knepley wrote:
> > 
> >> On Wed, May 25, 2022 at 11:55 AM Barry Smith <bsmith at petsc.dev> wrote:
> >> 
> >>> 
> >>>  It would be nice if when people received MR review requests it indicated
> >>> if the review was trivial and could be done in a minute or two. Then maybe
> >>> quick ones could flow through the system faster.
> >>> 
> >>>  How do people 1) know first when they have things they should (or have
> >>> been asked to) review and 2) know the list of things they should review at
> >>> any point in time. I am interested in other people's work flows to see if I
> >>> should adjust mine.
> >> 
> >> 
> >> I get emails for review, but it would be nice if there was a dashboard at
> >> Gitlab where we could see what is pending for us.
> > 
> > https://gitlab.com/dashboard/merge_requests?scope=all&state=opened&reviewer_username=knepley
> > 
> > You can easily access this via the options on the top-right (along with issues, todo-list)
> > 
> >> 
> >> Is there a way to indicate if a review is trivial or long?
> > 
> > Perhaps the author could add some text wrt what parts the reviewer should pay extra attention to.
> 
>   Yes but how would they provide the text in a way that is simple for the reviewer to process quickly, some text in the intro screen box would only be of limited use. 

In some sense the author can do the first round of review - i.e add in comments along the corresponding diff lines - requesting appropriate feedback (or furnishing more context) that can help reviewer look at those changes more closely. I've used this mode a few times..

Satish

> For example, if gitlab had a "Changes for you to look at" based on your code ownership records then people could process exactly what they need to and not have to wade through things that they want to ignore.
> 
> > 
> > However who would evaluate if the review is trivial or long? Even if the author thinks its trivial - the reviewer might evaluate it differently.
> 
>   Sure, but it would be a way for someone to avoid putting something off if the notification stated clearly this will likely take you very little time. If you look and see it will take a long time you can put it off.
> 
> > 
> > One related issue is - some MRs pack in way too many changes - where multiple MRs would be more appropriate. [and easier to review - as now some of them could be trivial - and others require more thought]
> 
>    True. But since people don't always quickly process trivial ones (since they don't know they are trivial) the trivial ones can hang around as long as big ones and require just as much nagging of reviewers. So having a better 1) indication of triviality 2) easy way to review just the parts you are qualified for might lead to people doing a better job of separating MR.

Ultimately each developer uses a different process - anything to help push the review process forward is good - but I don't think there is a simple alternative to "communication to reviewer and action/response from the reviewer" to get things moving..

Satish

> 
> > 
> > Satish
>