[llvm-dev] How to get a review for a patch?

[Eli Friedman via llvm-dev]

Comments inline.

> -----Original Message-----
> From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Ralf Jung via
> llvm-dev
> Sent: Tuesday, February 26, 2019 3:21 AM
> To: Shoaib Meenai <smeenai at fb.com>; llvm-dev <llvm-dev at lists.llvm.org>
> Subject: [EXT] Re: [llvm-dev] How to get a review for a patch?
> 
> Hi Shoaib,
> 
> > You added the old account for Eli (eli.friedman); I went ahead and switched it
> > to the newer account (efriedma). You can tell it's an old account because if
> you
> > go to https://reviews.llvm.org/p/eli.friedman/ (which can be accessed by e.g.
> > clicking the eli.friedman in your reviewers list), the last activity is from
> > 2016, whereas https://reviews.llvm.org/p/efriedma/ has recent activity.
> > Hopefully that gets you some activity.
> 
> Thanks a lot! That seems like an easy trap to run into. Is there a way to not
> suggest the old accounts (in the drop-down menu) when adding reviewers?

I don't think there's any way in general to avoid old accounts.  IIRC there might be a way I can mark that specific account as dead; I'll look into it.  That said, if llvm-commits is CC'ed, I have my filters set so I'll see it anyway.  It's my fault I didn't reply; I'm pretty sure I got the notification, it just got buried in other email.

If you don't get a review within a week, please reply to the patch (adding a comment "ping" is enough).  And if you don't get a reply in the week after that, escalating to llvmdev is the right thing to do.

> > It's also customary to add llvm-commits
> > as a subscriber instead of a reviewer, but that shouldn't make too much of a
> > difference.
> 
> Thanks, I'll try to remember this for next time.
> I did this based on the following text in [the
> docs](https://llvm.org/docs/Phabricator.html#phabricator-request-review-
> web):
> 
> > Add reviewers (see below for advice). (If you set the Repository field correctly,
> llvm-commits or cfe-commits will be subscribed automatically; otherwise, you
> will have to manually subscribe them.)
> 
> I was not aware of there being separate notions of "reviewers" and
> "subscribers", so with this being in the "Add reviewers" not I thought "to
> subscribe" meant "to add as a reviewer".  Actually from what I recall,
> llvm-commits had been added automatically (but I might misremember).
> (I hope this kind of feedback helps to improve the documentation.)

If you accidentally add llvm-commits as a reviewer instead of a subscriber, it doesn't really matter; Phabricator still sends the same email.  It's only really an issue if you forget to add it at all.

-Eli