Is it ok to Close PRs to indicate WiP?

[Andrew Wilkins]

On Tue, Jun 17, 2014 at 5:02 PM, John Meinel <john at arbash-meinel.com> 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 at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/juju-dev/attachments/20140617/2294821e/attachment.html>