DR workflows

[Joachim Breitner]

Hi

Am Donnerstag, den 16.02.2017, 17:01 -0500 schrieb Ben Gamari:
> > ¹ There is a workflow problem with Phab’s DR:
> >   * A creates a new DR.
> >   * B requests changes. DR now in state “revision needed”
> >   * C requests changes. DR still in state “revision needed”
> >   * A makes changes. DR now in state “needs review”
> >   * C looks at the changes and finds his concern addressed
> >     and accepts the revision. DR now in state “accepted”.
> >   * G comes along, sees a DR in state “accepted” and lands it.
> >   Problem: B did not have the chance to check the new revision.
> > 
> 
> Actually, the problem in this particular case was the Simon left
> comments but didn't request changes. Had he done so the Diff wouldn't
> have entered "accepted" state until he accepted.

Indeed! I thought I saw “Simon requested changes“ in this one, but my
memory was failing me here. So I partly retract my fussmaking here.


> While I understand why this is the case, it can be a bit unfortunate in
> the case of an open-source project, where a drive-by reviewer might
> leave a comment, the author makes the requested change, and the reviewer
> never returns to accept the change. In this case the Diff remains in a
> sort of limbo, even if someone else accepts it.

Would it not at least show up on Phab’s starting page, under “Ready to
Review” or some of the other categories. In the worst case, the author
can ping the reviewer here. I think it is fine that if someone requests
changes to expect him to follow up. The drive-by reviewer should leave
comments without requesting changes then.

> This is to some extent a social problem: In an ideal world reviewers
> would continue to submit reviews until the patch is merged. However, in
> the case of a project like GHC this is rarely the case. For this reason
> I sometimes need to ping reviewers and explicitly ask them to accept
> changes.

This is fine, I think.


What I sometimes miss as a reviewer is to indicate: “As far as I am
concerned, this looks good, but I did not review the whole thing”, for
example if one aspect of a change interacts with “my” code in GHC, and
I verified that this part is ok.

Greetings,
Joachim

-- 
Joachim “nomeata” Breitner
  mail at joachim-breitner.de • https://www.joachim-breitner.de/
  XMPP: nomeata at joachim-breitner.de • OpenPGP-Key: 0xF0FBF51F
  Debian Developer: nomeata at debian.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://mail.haskell.org/pipermail/ghc-devs/attachments/20170216/37b8c06e/attachment.sig>