[Reviewers] Cultural norms & phabricator comments?

[Durham Goode]

In our normal internal workflow, every update to a Diff is accompanied with a short message, like “Address Greg’s feedback” or “Update the try/finally to handle SystemError”.  This is enforced by having ‘arc diff’ (i.e. hg phabsend in our case), prompt the user when sending updates to a Diff.  I’m not sure how well that would work with the upstream pattern of sending updates to many commits at once though.

We could try the Done button. It’s not present on our internal phabricator so we haven’t used it much. 

On 9/15/17, 8:56 AM, "Augie Fackler" <raf at durin42.com> wrote:

    One of the things I've been struggling with is that it's not always clear to me when I should consider feedback addressed. With email-based patches, it's mostly unambiguous: a new patch on the list implicitly "resolves" all prior comments, and is a clean slate for acceptance.
    
    In phabricator, though, I find myself doubting things a lot. An example of this I've tripped on today is https://urldefense.proofpoint.com/v2/url?u=https-3A__phab.mercurial-2Dscm.org_D706&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=iPtmLDrbnLYewe4e-hrYcnQMROSjGBEF8xvySAdU7Yw&s=vY_GZAHSFCbEmkIBkrEfa3QOjx8J6S8zHZzrQhFLtB4&e=  <https://urldefense.proofpoint.com/v2/url?u=https-3A__phab.mercurial-2Dscm.org_D706&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=iPtmLDrbnLYewe4e-hrYcnQMROSjGBEF8xvySAdU7Yw&s=vY_GZAHSFCbEmkIBkrEfa3QOjx8J6S8zHZzrQhFLtB4&e= > - where I *suspect* durham has resolved indygreg's feedback, but that's not obvious to me either. Should we have a best practice that if you think you've resolved a comment you mark it as done? I feel like that'd help me a lot, not sure about other reviewers.