Roman Lebedev via llvm-dev
2019-Jun-19 14:29 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
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.
James Henderson via llvm-dev
2019-Jun-19 14:50 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
I don't think I've ever looked at llvm-commits for patches that need reviewing except for when I was looking to actively get involved in reviewing an area I wasn't already subscribed to by default. Certainly, if somebody posted a patch on llvm-commits to one of the binutils like llvm-readelf or llvm-objcopy, I'd be interested in it, but probably wouldn't see it nowadays, as I have too great a workload to filter through the mailing lists, whereas I can setup Herald rules to get myself auto-subscribed/added as a reviewer to relevant patches on Phabricator. Registering isn't a complicated process, compared to developing a patch in most cases anyway. That's a long way of saying +1. On Wed, 19 Jun 2019 at 15:30, 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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190619/53d8d8fb/attachment.html>
via llvm-dev
2019-Jun-19 15:49 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
(Resend with the list re-added) From: Robinson, Paul Sent: Wednesday, June 19, 2019 11:49 AM To: 'jh7370.2008 at my.bristol.ac.uk'; Roman Lebedev Subject: RE: [llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches Conversely, I always scan the mailing list, because I am incapable of navigating the Phabricator UI to find the relevant and interesting reviews. (People have tried to help me in the past. Maybe it's residual scarring from back when Phab was actively user-hostile; I admit that's better now. The "help" remains spectacularly un-helpful, last I looked.) There was talk in the past about connecting Phab up to GitHub somehow so any GitHub user could authenticate to Phab with no real effort, and as we're imminently moving to GitHub that seems like a really good idea. With that in place, the registration step effectively goes away, and I would have no further objection on those grounds. --paulr From: llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] On Behalf Of James Henderson via llvm-dev Sent: Wednesday, June 19, 2019 10:51 AM To: Roman Lebedev Cc: llvm-dev Subject: Re: [llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches I don't think I've ever looked at llvm-commits for patches that need reviewing except for when I was looking to actively get involved in reviewing an area I wasn't already subscribed to by default. Certainly, if somebody posted a patch on llvm-commits to one of the binutils like llvm-readelf or llvm-objcopy, I'd be interested in it, but probably wouldn't see it nowadays, as I have too great a workload to filter through the mailing lists, whereas I can setup Herald rules to get myself auto-subscribed/added as a reviewer to relevant patches on Phabricator. Registering isn't a complicated process, compared to developing a patch in most cases anyway. That's a long way of saying +1. On Wed, 19 Jun 2019 at 15:30, 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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190619/33f31ef0/attachment.html>
Reid Kleckner via llvm-dev
2019-Jun-19 17:50 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
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. 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 >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190619/26b400f8/attachment.html>
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>
Doerfert, Johannes via llvm-dev
2019-Jul-11 16:45 UTC
[llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches
+1 for clarity as it is friendlier for outsiders ________________________________________ From: llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf of Roman Lebedev via llvm-dev <llvm-dev at lists.llvm.org> Sent: Wednesday, June 19, 2019 09:29 To: llvm-dev at lists.llvm.org Subject: [llvm-dev] [RFC] Documentation clarification: Phabricator, not the lists is the main entry point for new patches 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