Finkel, Hal J. via llvm-dev
2019-Jun-19 17:57 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
On 6/19/19 12:50 PM, Reid Kleckner via llvm-dev wrote: I believe the history is that when Phab was initially introduced, we wrote the documentation this way to make things easy for reviewers who didn't want to change their workflow. But, I agree with your observations. The majority of code review seems to happen on Phabricator, and the best way to get traction on a new patch is to upload it to Phab and add a few reviewers by name. Regardless of what workflow reviewers would prefer, I think the documentation should recommend Phabricator over email to first time contributors, since, in my experience, it gets better results. +1 -Hal On Wed, Jun 19, 2019 at 7:30 AM Roman Lebedev via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote: The current documentation talks about both the Phabricator review, and review as mail replies on -commits lists. It also talks about submitting patches to lists, with the subtext that it may be friendlier for outsiders. It is true that Phabricator has some entry threshold, larger than github, or maillists, so the attempt is not unwarranted. But from what i can tell, 99.9% patches go via Phabricator. There is a large chance that such a mail-only patch will simply be overlooked, ignored, or the very first reply will be "Please post the patch to Phabricator". Both of these cases i would call counter-welcoming. I don't think that is what we want? I propose to fix the docs to specify that all new patches should go via Phabricator, not lists: https://reviews.llvm.org/D63488 Roman. _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev _______________________________________________ LLVM Developers mailing list llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190619/e64f8ac9/attachment.html>
Philip Reames via llvm-dev
2019-Jun-19 20:46 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
+1 With one important note which is that the documentation should not that authors are expected to watch llvm-commit for responses, since not all of them make it to phabricator. And definitely emphasize the need to add llvm-commits explicitly to the review! Philip On 6/19/19 10:57 AM, Finkel, Hal J. via llvm-dev wrote:> On 6/19/19 12:50 PM, Reid Kleckner via llvm-dev wrote: >> I believe the history is that when Phab was initially introduced, we >> wrote the documentation this way to make things easy for reviewers >> who didn't want to change their workflow. But, I agree with your >> observations. The majority of code review seems to happen on >> Phabricator, and the best way to get traction on a new patch is to >> upload it to Phab and add a few reviewers by name. Regardless of what >> workflow reviewers would prefer, I think the documentation should >> recommend Phabricator over email to first time contributors, since, >> in my experience, it gets better results. > > > +1 > > -Hal > > >> >> On Wed, Jun 19, 2019 at 7:30 AM Roman Lebedev via llvm-dev >> <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: >> >> The current documentation talks about both the Phabricator >> review, and review >> as mail replies on -commits lists. It also talks about submitting >> patches to lists, >> with the subtext that it may be friendlier for outsiders. >> >> It is true that Phabricator has some entry threshold, larger than >> github, or maillists, >> so the attempt is not unwarranted. But from what i can tell, >> 99.9% patches go >> via Phabricator. There is a large chance that such a mail-only patch >> will simply be >> overlooked, ignored, or the very first reply will be "Please post >> the patch to >> Phabricator". >> >> Both of these cases i would call counter-welcoming. >> I don't think that is what we want? >> >> I propose to fix the docs to specify that all new patches should go >> via Phabricator, not lists: >> https://reviews.llvm.org/D63488 >> >> Roman. >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190619/b1bc33df/attachment.html>
James Henderson via llvm-dev
2019-Jun-20 09:06 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
> > And definitely emphasize the need to add llvm-commits explicitly to the > review! >If I'm not mistaken, there's a Herald rule that now does this. Also, make sure llvm-commits is added as a subscriber, not a reviewer! On Wed, 19 Jun 2019 at 21:46, Philip Reames via llvm-dev < llvm-dev at lists.llvm.org> wrote:> +1 > > With one important note which is that the documentation should not that > authors are expected to watch llvm-commit for responses, since not all of > them make it to phabricator. And definitely emphasize the need to add > llvm-commits explicitly to the review! > > Philip > On 6/19/19 10:57 AM, Finkel, Hal J. via llvm-dev wrote: > > On 6/19/19 12:50 PM, Reid Kleckner via llvm-dev wrote: > > I believe the history is that when Phab was initially introduced, we wrote > the documentation this way to make things easy for reviewers who didn't > want to change their workflow. But, I agree with your observations. The > majority of code review seems to happen on Phabricator, and the best way to > get traction on a new patch is to upload it to Phab and add a few reviewers > by name. Regardless of what workflow reviewers would prefer, I think the > documentation should recommend Phabricator over email to first time > contributors, since, in my experience, it gets better results. > > > +1 > > -Hal > > > > On Wed, Jun 19, 2019 at 7:30 AM Roman Lebedev via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> The current documentation talks about both the Phabricator review, and >> review >> as mail replies on -commits lists. It also talks about submitting >> patches to lists, >> with the subtext that it may be friendlier for outsiders. >> >> It is true that Phabricator has some entry threshold, larger than >> github, or maillists, >> so the attempt is not unwarranted. But from what i can tell, 99.9% >> patches go >> via Phabricator. There is a large chance that such a mail-only patch >> will simply be >> overlooked, ignored, or the very first reply will be "Please post the >> patch to >> Phabricator". >> >> Both of these cases i would call counter-welcoming. >> I don't think that is what we want? >> >> I propose to fix the docs to specify that all new patches should go >> via Phabricator, not lists: >> https://reviews.llvm.org/D63488 >> >> Roman. >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> > > _______________________________________________ > LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory > > > _______________________________________________ > LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190620/3144742a/attachment.html>