[llvm-dev] [cfe-dev] RFC: Code Review Process

Raphael Isemann via llvm-dev llvm-dev at lists.llvm.org
Thu Oct 7 03:45:33 PDT 2021


Just two comments on the process here:

1. I don't think we can really make a 'wrong' decision regarding what
code review platform we use. While Github's review interface has its
own problems, it's not so bad that it would actually prevent people
from reviewing code. Having said that, the only way we could mess up
is if we end up with the situation that James describes, where we have
two groups in the community using different review platforms. And the
only way I can see this could happen is if we do the same thing we did
with Discord or Discourse where we just push some new system but we
don't have the consensus or critical mass to make everyone switch.
Both Discord and Discourse were IMHO good ideas in general and were
well intentioned, but the way we switched to them split & hurt the
community in the end. Given how critical code review is, I would
rather wait until we have enough consensus for the switch than just do
it and hope it works out well.

2. I do appreciate that the foundation/infrastructure group is trying
to improve our infrastructure, but I am not sure why the code review
system has become an apparently urgent issue. While Phabricator is
clearly going the way of the Dodo, from my own experience it at least
still works. Meanwhile both the mailing list and the bug tracker are
pretty much completely broken (the mailing list in the weird legal
spam limbo and the bugtracker having a disabled user registration). I
don't want to be the person that tells other people what to do, but if
we have resources/time to spend on fixing up our infrastructure, I
hope we could spend it on fixing the things that currently don't work.
Having a working bug tracker or some new mailing list system where I
(and other moderators) don't have to manually approve messages 24/7
and actually accept subscribers would be nice.

Thanks,
- Raphael

Am Mi., 6. Okt. 2021 um 19:34 Uhr schrieb Philip Reames via cfe-dev
<cfe-dev at lists.llvm.org>:
>
> Since I think we're risking confusion on the point here, let me clarify that at least my response to this thread should not be read as opposition (or support) for a migration to github.  I am expressing no opinion on that matter.  I see the primary point being discussed in this thread being the decision making process proposed, not the decision itself.
>
> Philip
>
> On 10/6/21 10:26 AM, Chris Tetreault via llvm-dev wrote:
>
> > … nothing's really changed from the previous conversations on PRs versus Github, apart from the announcement of end of support by the upstream company, but that was quite a while ago now, and even with the stale Arcanist issue, there hasn't been a big push from community members to change …
>
>
>
> James, If you’ll forgive me for cherry-picking a small part of your point, I think it bears mentioning that human beings tend to ignore future problems until they become current problems. Most of us here want to work on compilers, not deal with infrastructure. This doesn’t mean that the status quo is ok.
>
>
>
> As I see it, it would be a mistake to just continue on with zombie-phabricator as we have. Perhaps the board of directors could have taken a different tone when presenting this issue, but I think they are doing the right thing by forcing a change soon. Tools are degrading, and security fixes are not being implemented. If we do nothing we’re all going to wake up some day and find that the github repo has had its owner changed or somesuch catastrophe. We need to do *something*, and I think setting a deadline for a change was the right call.
>
>
>
> From my perspective, there are 4 reasonable things we can do, in order of disruptiveness:
>
>
>
> Investigate a community replacement for phabricator. Does Phorge meet our needs? Is there a maintained fork of phabricator? Can we just drop in some replacement?
> Fork Phabricator, and take on the maintenance burden ourselves. This sounds like work.
> Move to github PRs. As others have mentioned, there are pros and cons to this.
> Something else? We’d have to figure out what this is, and justify it over options 1-3.
>
>
>
> If the deadline the board has set is unpalatable to the community, then perhaps it makes sense to fork Phabricator, and then decide on a longer term migration plan. But we need to do something and we need to do it now, not when there’s an actual fire.
>
>
>
> Personally, I like Phabricator, and find github PRs to be tedious to work with. If we went with github PRs, I would be able to work, but I would prefer something more like phabricator.
>
>
>
> thanks,
>
>    Chris Tetreault
>
>
>
> From: cfe-dev <cfe-dev-bounces at lists.llvm.org> On Behalf Of James Henderson via cfe-dev
> Sent: Wednesday, October 6, 2021 1:47 AM
> To: Tanya Lattner <tanyalattner at llvm.org>
> Cc: llvm-dev <llvm-dev at lists.llvm.org>; Renato Golin <rengolin at gmail.com>; clang developer list <cfe-dev at lists.llvm.org>; openmp-dev (openmp-dev at lists.llvm.org) <openmp-dev at lists.llvm.org>; LLDB Dev <lldb-dev at lists.llvm.org>
> Subject: Re: [cfe-dev] [llvm-dev] RFC: Code Review Process
>
>
>
> WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros.
>
> Forgive me if I'm wrong, but if the community consensus is that we should continue to use Phabricator, and Phabricator is not being provided/maintained by the LLVM Foundation, isn't it moot what the LLVM Foundation/Infrastructure Working Group recommends/wants to happen? The current maintainers would continue to maintain Phabricator (assuming they wanted to), and people would still be able to review things there. What would happen if the Foundation officially supported PRs, without community consensus (in particular from the Phabricator maintainers), is a potential split in the community, with some continuing in the old way and others using the new way (and presumably some choosing to review on both platforms). This cannot be good.
>
>
>
> I'm all for the discussion to be had, about whether we switch, but as far as I can see, nothing's really changed from the previous conversations on PRs versus Github, apart from the announcement of end of support by the upstream company, but that was quite a while ago now, and even with the stale Arcanist issue, there hasn't been a big push from community members to change: the consensus in the posts discussing this and the moving to PRs seems to still be "there are things that are blocking switching still".
>
>
>
> At the most, from this IWG/Foundation consultation, it should be that the Foundation recommends one or other approach, and is willing to provide X infrastructure required. The community can then choose to agree with whatever approach is recommended or stick with the status quo. There shouldn't be an edict that says we will do one thing or the other.
>
>
>
> Another side-point: whilst the IWG may consist of people who care about LLVM, there are far more people who care as much, but who just don't have the time to participate in such a group. This is particularly important to note, because the community does not elect members to this group. To an extent, the same is also true of the Foundation board itself, since there are plenty of people who may not agree with their decisions, but don't have the time to volunteer for the board. I'm not suggesting that there's any malice in this discussion, and indeed, the fact that it's open to community comments certainly is helpful, but I'd be worried of some kind of echo chamber/unconscious bias within the small groups suggesting there is consensus for one approach, when the wider community thinks otherwise.
>
>
>
> James
>
>
>
> On Tue, 5 Oct 2021 at 20:52, Tanya Lattner via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
> Hello! The purpose of this email is to start a discussion about our code review tools. No decisions have been made about changing tools. The idea behind a timeline is so that information could be gathered in a timely manner. The Infrastructure Working Group was formed to bring together community members who have an experience and/or passion regarding infrastructure. Anyone can participate in this working group and like the LLVM Foundation, the minutes are all made public.
>
>
>
> The LLVM Foundation’s mission is to support the LLVM project and help ensure the health and productivity of of the community and this is done through numerous ways including infrastructure. I do not think it is a negative thing that the foundation board of directors would be discussing our current tools and gathering information how how well they work and how we can make them better. As the legal entity who bares financial and legal responsibility for a lot of the infrastructure, this would make sense. This also makes sense because of the people involved who care a lot about LLVM and the project. But, the LLVM Foundation does not pay for Phabricator and we are very grateful for Google’s support of this critical piece of our infrastructure.
>
>
>
> Regarding Phabricator, there are a couple of pieces of information that were provided to the LLVM Foundation by maintainers (maybe previous it sounds like) of this instance and how we may need to look into alternative ways to support it.  In addition, Phacility itself has publicly stated that it is winding down operations. (https://admin.phacility.com/phame/post/view/11/phacility_is_winding_down_operations/). Lastly, there are questions about why we are not using GitHub pull requests as we are on GitHub and that might be the natural path to take for a number of reasons.
>
>
>
> The above reasons are why the RFC was written. Perhaps it wasn’t written in the best way, but I also feel like it is being read in a negative way which is incredibly disappointing given I don’t feel there is a valid reason for this.
>
>
>
> -Tanya
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Oct 5, 2021, at 11:35 AM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
>
>
> On Tue, 5 Oct 2021 at 19:16, Tom Stellard <tstellar at redhat.com> wrote:
>
> However, it's not a good position for the Board to be responsible
> for something that it doesn't have control over.  If Google decided to stop hosting
> Phabricator for some reason (unlikely, but not impossible), the Board would be
> responsible for finding a replacement.
>
>
>
> Sorry, this is a very weak reason for such a strong worded "RFC".
>
>
>
> I _cannot_ imagine "Google" stopping to support something so quickly as to leave the foundation without recourse. And even if they did, *no one* would blame the foundation for that.
>
>
>
> Even if you ignore all the effort that hundreds of their engineers have done over the past decade to the project, this would hurt Google more than anyone else. It's a far fetched concern.
>
>
>
> And if the foundation wants "control" of a piece of infrastructure that Google has been maintaining for years, then this is a different discussion. Hopefully one that doesn't involve unilateral decisions.
>
>
>
> The main risk is that Phabricator is no longer maintained upstream.
> There was already an issue[1] recently where the arc tool stopped working and won't
> be fixed upstream. Using unmaintained software is a bigger risk.
>
>
>
> I don't like using unmaintained software either, but I think our Phab has had more attention than the upstream project. And no one has to use arc, I certainly never have.
>
>
>
> Don't get me wrong, I don't like Phab and I think Github would bring new people to the project, but it's gotta be done the right way, and pushing it isn't it.
>
>
>
> We, meaning the LLVM Board of Directors.  And really the problem isn't the self-hosting
> so much as it's the lack of an enforceable maintenance agreement the Foundation and the
> maintainers.
>
>
>
> The problem isn't self-hosting at all, given that Google is doing that.  (apologies, I assumed otherwise earlier).
>
>
>
> Neither is maintenance, given Google is doing that too.
>
>
>
> The only thing that's left is control, and I don't really understand why this is important, as I explained above.
>
>
>
> cheers,
>
> --renato
>
> _______________________________________________
> LLVM Developers mailing list
> 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
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


More information about the llvm-dev mailing list