On 13 October 2016 at 08:36, Andreas Mueller <[email protected]> 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 > [email protected] > https://mail.python.org/mailman/listinfo/scikit-learn > _______________________________________________ scikit-learn mailing list [email protected] https://mail.python.org/mailman/listinfo/scikit-learn
