Re: [scikit-learn] Permission for creating new labels

[Nelle Varoquaux]

On 13 October 2016 at 08:36, Andreas Mueller  wrote:
> going to the mailing list
>
> On 10/13/2016 01:35 AM, Raghav R V wrote:
>
> Thanks for the messages {Ga|Jo}el. ;)
>
>> We can use "needs second review" as an alternative to "MRG+1" but I don't
>> see the point of using both.
>
> I see the system of MRG+1 and MRG+2 as a more robust way of tracking
> approvals to see if the PR can be merged (I'm not sure if review approvals
> completely replace this?) and "Needs 2nd Review" as a quick way to search...
> "Needs 2nd Review" could also be used with MRG PRs which have already
> received a solid review and would need a 2nd look from those who don't have
> much time to do a full fledged review...
>
>> By the way: this discussion should happen on the ML.
>
> Sorry for that. I wasn't sure if this was a very useful/non-trivial
> suggestion and wanted to avoid noise there...
>
>> "Needs triage":
>
> I see that we have "Stale" label for that.
>
> I just added this to make it easier to find PRs to review.
> I'm not sure if it is not redundant with the "needs contrib" tag on a PR.
> I used "stale" if I was not sure if it's worth working on something and the
> author didn't respond for a while.
> I haven't used it a lot yet.
>
> I'm ambivalent about adding a "approved by one" (which I think is more
> explicit then "need one more") tag.
>
> You can search for PRs and issues without comments - I recently did that to
> make sure everything had at least one ;)
> I'm not sure you can search for the absence of tags. But I am planning to go
> through all issues tomorrow to see stuff
> that I have missed. I'll be catching up today with all notifications that I
> missed this year because writing my ;)
>
> Maybe having an list of statuses for PRs and issues that covers the common
> cases would be good, we just kind of had that discussion, right?
>
> Issues can be bug|enhancement|new feature with status needs contributor, has
> PR or needs confirmation/discussion. It would be nice to see
> if a issue has a PR, I think there is no way to do that from the search.
>
> PRs need changes or reviews or are stalled (which is "needs changes" for a
> long time and no response) and then might "need contributor".
>
>
> We could use "needs review" on issues and add a "has PR" tag for issues and
> a "one approval" tag for PRs.
>
> I agree with Joel that switching between "needs review" and "needs changes"
> in a currently active PR is likely to be cumbersome.

From my experience on matplotlib that has such a system, it is a not a
very good idea… Reviewers rarely change the tag to needs change, and
when they do, reviewers ignore it and continue reviewing it (which is
slightly annoying in some cases).

>
> ___
> scikit-learn mailing list
> scikit-learn@python.org
> https://mail.python.org/mailman/listinfo/scikit-learn
>
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn

Re: [scikit-learn] Permission for creating new labels

[Andreas Mueller]

going to the mailing list

On 10/13/2016 01:35 AM, Raghav R V wrote:

Thanks for the messages {Ga|Jo}el. ;)

> We can use "needs second review" as an alternative to "MRG+1" but I 
don't see the point of using both.


I see the system of MRG+1 and MRG+2 as a more robust way of tracking 
approvals to see if the PR can be merged (I'm not sure if review 
approvals completely replace this?) and "Needs 2nd Review" as a quick 
way to search... "Needs 2nd Review" could also be used with MRG PRs 
which have already received a solid review and would need a 2nd look 
from those who don't have much time to do a full fledged review...


>By the way: this discussion should happen on the ML.

Sorry for that. I wasn't sure if this was a very useful/non-trivial 
suggestion and wanted to avoid noise there...


> "Needs triage":

I see that we have "Stale" label for that.


I just added this to make it easier to find PRs to review.
I'm not sure if it is not redundant with the "needs contrib" tag on a PR.
I used "stale" if I was not sure if it's worth working on something and 
the author didn't respond for a while.

I haven't used it a lot yet.

I'm ambivalent about adding a "approved by one" (which I think is more 
explicit then "need one more") tag.


You can search for PRs and issues without comments - I recently did that 
to make sure everything had at least one ;)
I'm not sure you can search for the absence of tags. But I am planning 
to go through all issues tomorrow to see stuff
that I have missed. I'll be catching up today with all notifications 
that I missed this year because writing my ;)


Maybe having an list of statuses for PRs and issues that covers the 
common cases would be good, we just kind of had that discussion, right?


Issues can be bug|enhancement|new feature with status needs contributor, 
has PR or needs confirmation/discussion. It would be nice to see

if a issue has a PR, I think there is no way to do that from the search.

PRs need changes or reviews or are stalled (which is "needs changes" for 
a long time and no response) and then might "need contributor".



We could use "needs review" on issues and add a "has PR" tag for issues 
and a "one approval" tag for PRs.


I agree with Joel that switching between "needs review" and "needs 
changes" in a currently active PR is likely to be cumbersome.
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn