Re: Is it ok to Close PRs to indicate WiP?
I think you accidentally replied to just me, so I'm including juju-dev in my reply. On Tue, Jun 17, 2014 at 3:53 PM, Wayne Witzel wrote: > > > > On Tue, Jun 17, 2014 at 5:02 AM, John Meinel > wrote: > >> Since we are now trying to have everyone regularly rotate into a on-call >> reviewer day, and one of the goals of OCR is that you should try to touch >> all open reviews. However, I'm finding a bunch of things that have already >> been reviewed quite thoroughly and look much more like we are just waiting >> for the person to do what was requested and then ask for review again. >> >> In Launchpad, we used Work in Progress to indicate this. I don't see any >> equivalent on Github (you just have whether the current PR is open or >> closed). I'm a little concerned that just Closing a request is going to >> make it easy for the person who submitted it forget about it. However, I >> also don't think we want all reviewers to have to poll through a large >> backlog every day. >> > > I've already seen some people changing the title of the pull request to > WIP: and then back after they are ready for review again, we could > make that convention? > > I think that is going to work better. I didn't realize that I only had "Close" rights because all the team leads are still superusers on the juju/juju project. So if general reviewers and submitters can set it to WIP, then we need another process. Editing the title seems ok here. > >> I suppose a meta question exists, why do we have such a huge pile of >> things that have been reviewed but not actually responded to by the >> original person? >> >> Also, I do think we want to follow our old Rietveld behavior, where for >> each comment a reviewer made, the submitter can respond (even if just with >> "Done"). I realize this generates a lot of email noise, but it means that >> any reviewer can come along and see what has been addressed and what >> hasn't. Or at least follow along with the conversation. >> > >> > +1 > >> >> Thoughts? Is Closed to big of a hammer. Is there something else in our >> process that we need to focus on? >> > Where are we at with moving to another review system all together? I think > expediting that process should be a focus. > I asked Ian, and he said Martin is currently looking into it. That still probably means a week or so before it would actively be useful. John =:-> -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Is it ok to Close PRs to indicate WiP?
On 17 June 2014 10:02, John Meinel wrote: > Also, I do think we want to follow our old Rietveld behavior, where for each > comment a reviewer made, the submitter can respond (even if just with > "Done"). I realize this generates a lot of email noise, but it means that > any reviewer can come along and see what has been addressed and what hasn't. > Or at least follow along with the conversation. I agree entirely. This is even more important since github doesn't make it possible to see what changes have been made in response to a given comment. > Thoughts? Is Closed to big of a hammer. Is there something else in our > process that we need to focus on? I think that only the person that created the pull request should close it, unless it has been merged. Unfortunately I can't think of a decent way of finding PRs that still need review. Perhaps someone could hack up a quick tool that pulls comments from outstanding PRs and prints any PRs that don't have a "reviewed" comment. http://godoc.org/github.com/google/go-github/github#PullRequestsService.ListComments -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Re: Is it ok to Close PRs to indicate WiP?
On Tue, Jun 17, 2014 at 5:02 PM, John Meinel wrote: > Since we are now trying to have everyone regularly rotate into a on-call > reviewer day, and one of the goals of OCR is that you should try to touch > all open reviews. However, I'm finding a bunch of things that have already > been reviewed quite thoroughly and look much more like we are just waiting > for the person to do what was requested and then ask for review again. > I found the same thing when I was OCR last week. It's not great. > In Launchpad, we used Work in Progress to indicate this. I don't see any > equivalent on Github (you just have whether the current PR is open or > closed). I'm a little concerned that just Closing a request is going to > make it easy for the person who submitted it forget about it. However, I > also don't think we want all reviewers to have to poll through a large > backlog every day. > > I suppose a meta question exists, why do we have such a huge pile of > things that have been reviewed but not actually responded to by the > original person? > > Also, I do think we want to follow our old Rietveld behavior, where for > each comment a reviewer made, the submitter can respond (even if just with > "Done"). I realize this generates a lot of email noise, but it means that > any reviewer can come along and see what has been addressed and what > hasn't. Or at least follow along with the conversation. > Yes, please, let's do this. Then we can have some confidence that the person has done (or not done with a reason) what was suggested. > Thoughts? Is Closed to big of a hammer. Is there something else in our > process that we need to focus on? > Unfortunately I don't think it'll work well. As I do now have rights to commit, I don't have the ability to Close. Another reason to move to some other review service, IMO. John > =:-> > > > -- > Juju-dev mailing list > Juju-dev@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > > -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
Is it ok to Close PRs to indicate WiP?
Since we are now trying to have everyone regularly rotate into a on-call reviewer day, and one of the goals of OCR is that you should try to touch all open reviews. However, I'm finding a bunch of things that have already been reviewed quite thoroughly and look much more like we are just waiting for the person to do what was requested and then ask for review again. In Launchpad, we used Work in Progress to indicate this. I don't see any equivalent on Github (you just have whether the current PR is open or closed). I'm a little concerned that just Closing a request is going to make it easy for the person who submitted it forget about it. However, I also don't think we want all reviewers to have to poll through a large backlog every day. I suppose a meta question exists, why do we have such a huge pile of things that have been reviewed but not actually responded to by the original person? Also, I do think we want to follow our old Rietveld behavior, where for each comment a reviewer made, the submitter can respond (even if just with "Done"). I realize this generates a lot of email noise, but it means that any reviewer can come along and see what has been addressed and what hasn't. Or at least follow along with the conversation. Thoughts? Is Closed to big of a hammer. Is there something else in our process that we need to focus on? John =:-> -- Juju-dev mailing list Juju-dev@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
