[llvm-dev] How to get a review for a patch? (original) (raw)

Eli Friedman via llvm-dev llvm-dev at lists.llvm.org
Tue Feb 26 11:34:44 PST 2019


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 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



More information about the llvm-dev mailing list