Github PR Assignee

Hello

I'd like to suggest small improvement in our PR reviewing process. I think it
but it would be nice if reviewers assigned themselves as assignee of a PR once
they start reviewing and plan to do the full review.

The motivation is that sometimes I get (valuable) comments from someone who
won't finish the full review because they just spotted something from their
area. Which makes the status of PR a bit unclear not only for me (should I
wait for more comments from the same reviewer and ping him if I don't get
them?) but also for other reviewers (should I start reviewing while someone
else might have started already?).

I see that lot of closed PRs are already assigned so somehow we already use
it, but most of open PRs are not assigned to anyone. So, how do you feel about
using this field?

If we mostly agree, I don't think we'd need to formalize it, maybe I would
only mention that in handbook#Codereviews.

Both +1 and -1 are welcome

··· -- Marek

> Hello
>
> I'd like to suggest small improvement in our PR reviewing process. I think it
> but it would be nice if reviewers assigned themselves as assignee of a PR
> once
> they start reviewing and plan to do the full review.
>
> The motivation is that sometimes I get (valuable) comments from someone who
> won't finish the full review because they just spotted something from their
> area. Which makes the status of PR a bit unclear not only for me (should I
> wait for more comments from the same reviewer and ping him if I don't get
> them?) but also for other reviewers (should I start reviewing while someone
> else might have started already?).

If I understand that correctly, this reviewer with just random comment would
not assign themselves to the PR, just those doing end2end, right?

– Ivan

··· ----- Original Message -----

I see that lot of closed PRs are already assigned so somehow we already use
it, but most of open PRs are not assigned to anyone. So, how do you feel
about
using this field?

If we mostly agree, I don’t think we’d need to formalize it, maybe I would
only mention that in handbook#Codereviews.

Both +1 and -1 are welcome


Marek


You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Yeah. Only the reviewer that commits to do end2end review.

··· On Thursday 31 of March 2016 09:10:29 Ivan Necas wrote: > ----- Original Message ----- > > > Hello > > > > I'd like to suggest small improvement in our PR reviewing process. I think > > it but it would be nice if reviewers assigned themselves as assignee of a > > PR once > > they start reviewing and plan to do the full review. > > > > The motivation is that sometimes I get (valuable) comments from someone > > who > > won't finish the full review because they just spotted something from > > their > > area. Which makes the status of PR a bit unclear not only for me (should I > > wait for more comments from the same reviewer and ping him if I don't get > > them?) but also for other reviewers (should I start reviewing while > > someone > > else might have started already?). > > If I understand that correctly, this reviewer with just random comment would > not assign themselves to the PR, just those doing end2end, right?


Marek

– Ivan

I see that lot of closed PRs are already assigned so somehow we already
use
it, but most of open PRs are not assigned to anyone. So, how do you feel
about
using this field?

If we mostly agree, I don’t think we’d need to formalize it, maybe I would
only mention that in handbook#Codereviews.

Both +1 and -1 are welcome


Marek


You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

> I'd like to suggest small improvement in our PR reviewing process. I think it
> but it would be nice if reviewers assigned themselves as assignee of a PR once
> they start reviewing and plan to do the full review.

That's what I do, did not have enough time to do core reviews in last
couple of weeks tho.

The plan have one problem tho, you can't assign yourself if you don't
have permissions I think. I don't remember which permission is that, but
it won't work for everybody.

In that case, I'd just recommend to show the commitment as an comment
and somebody else can assign the PR.

··· -- Later, Lukas #lzap Zapletal

Thanks for explanation, +1 makes sense

– Ivan

··· ----- Original Message ----- > +1 to this, in general. > http://dashboard-ehelms.rhcloud.com/dashboard/reviews shows the assigned > reviewer as well, which helps with filtering, and once it's a common > practice, I'd be happy to produce some reviewer stats ;) > > Greg > > -- > You received this message because you are subscribed to the Google Groups > "foreman-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to foreman-dev+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. >

You need commit rights to be assigned, either to assign yourself or to
be assigned by another. It requires commit rights to merge anyway, so
it's pretty much fine.

··· On 31/03/16 14:45, Lukas Zapletal wrote: >> I'd like to suggest small improvement in our PR reviewing process. I think it >> but it would be nice if reviewers assigned themselves as assignee of a PR once >> they start reviewing and plan to do the full review. > > That's what I do, did not have enough time to do core reviews in last > couple of weeks tho. > > The plan have one problem tho, you can't assign yourself if you don't > have permissions I think. I don't remember which permission is that, but > it won't work for everybody. > > In that case, I'd just recommend to show the commitment as an comment > and somebody else can assign the PR.


Dominic Cleal
dominic@cleal.org

+1 to this, in general.
http://dashboard-ehelms.rhcloud.com/dashboard/reviews shows the assigned
reviewer as well, which helps with filtering, and once it's a common
practice, I'd be happy to produce some reviewer stats :wink:

Greg