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?> 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.) Kind regards, Ralf> > > > The standard procedure is also to ping reviews weekly, which helps get review > activity in most cases. > > > > *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Ralf Jung via > llvm-dev <llvm-dev at lists.llvm.org> > *Reply-To: *Ralf Jung <post at ralfj.de> > *Date: *Monday, February 25, 2019 at 9:03 AM > *To: *llvm-dev <llvm-dev at lists.llvm.org> > *Subject: *[llvm-dev] How to get a review for a patch? > > > > Hi all, > > > > I submitted a small documentation patch [1] three weeks ago. This is my first > > patch submitted against LLVM, so I was extra careful to follow the instructions > > (turns out this is quite a bit more complicated than the pull-request-based > > workflow). So far, I got no reaction at all to my patch. Does that mean I did > > something wrong, like picking the wrong reviewer? Should I pick another one? > > Obviously I don't know the code organization nor any of the people involved, so > > I don't feel I can meaningfully judge who might be able to review this patch. > > > > [1]: > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D57600&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=geYJV3f7Jpb0hmOUToiU6RGGPLWOQ7wWwah_AlGL2_4&s=G82iq_IAHTCK264gxt8V7oXf20Ppt7zmAzr53wWGpks&e> > > > Kind regards, > > Ralf > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Ddev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=geYJV3f7Jpb0hmOUToiU6RGGPLWOQ7wWwah_AlGL2_4&s=zn_t2nciTI6584SuUNb5yuFVbzfaJ1qrHco7ELwMRxI&e> > >
Eli Friedman via llvm-dev
2019-Feb-26 19:34 UTC
[llvm-dev] How to get a review for a patch?
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
Florian Hahn via llvm-dev
2019-Feb-26 23:08 UTC
[llvm-dev] How to get a review for a patch?
Hi,> On Feb 26, 2019, at 3:20 AM, Ralf Jung via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > 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? > >> 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.)There’s https://llvm.org/docs/Contributing.html <https://llvm.org/docs/Contributing.html> which would ideally contain all relevant info, but it might be hard to discover. Did you have a look at that page? Cheers, Florian -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190226/11992a8f/attachment.html>
Hi,>> 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.) > > There’s https://llvm.org/docs/Contributing.html which would ideally contain all > relevant info, but it might be hard to discover. Did you have a look at that page?I had found that, yes, bit it took me a while. It is from there that I reached <https://llvm.org/docs/Phabricator.html#phabricator-request-review-web>. I started at <https://llvm.org/> and scanned for "contribute", and that went nowhere at first. Even when I found <https://llvm.org/docs/> it wasn't at all obvious that this was the right direction, the contribution documentation is *way* down at the bottom. Given that <https://github.com/llvm/llvm-project/> is on GH, it might be worth adding a CONTRIBUTING.md or at least adding some pointers to the README -- that's actually the place I started looking for information. Kind regards, Ralf