[llvm-dev] Enable Contributions Through Pull-request For LLVM

Sam Elliott via llvm-dev llvm-dev at lists.llvm.org
Mon Nov 11 08:47:13 PST 2019


I have a few reactions to this proposal

The first is that I do think changing too many parts of the project infrastructure in a short space of time is not sensible. LLVM has just finished a big SVN to git migration, which for builders and other tooling hasn’t been particularly easy. It’s not clear that the tooling and documentation has entirely caught up with this migration, but I’m sure it will. There has been a proposal to move away from bugzilla, which does seem like a pressing concern, of more importance.

Secondly, I do not think the github pull request system is nearly as good as phabricator’s patch review system. I use “patch history” during every single review (something github entirely lacks), as well as the ability to mark parent and child patches. I also find the comment interface on phabricator to accurately show all the discussion and feedback, both in context (inline comments), and chronologically. In contrast, I regularly find github decides not to load comments, or marks them as resolved before they are (and hides them). I do not wish the quality of LLVM’s codebase to be beholden to the choices github makes around which parts of the discussion to show me. And the issues with subscribing to github issues carry over to subscribing to github pull requests.

I understand that this proposal, in its original form, does not deprecate Phabricator, but it seems like the discussion has moved far closer towards this point. 

In a more general point, some of these major decisions are very hard to gauge the sentiment through the mailing list. I know that some other communities run yearly surveys to understand the state of the ecosystem and their community’s feelings on some of the more far-reaching changes to the project - maybe now is a good moment for one, given we have so many proposals up for consideration (this one, github issues, the build system simplification)?

Sam

> On 7 Nov 2019, at 5:32 am, Mehdi AMINI via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> 
> Hi all,
> 
> Now that we're on GitHub, we can discuss about pull-requests.
> I'd like to propose to enable pull-request on GitHub, as a first step as an experimental channel alongside the existing methods for contributing to LLVM.
> This would allow to find workflow issues and address them, and also LLVM contributors in general to start getting familiar with pull-requests without committing to switching to pull-requests immediately. The community should evaluate after a few months what would the next steps be.
> 
> GitHub pull-requests is the natural way to contribute to project hosted on GitHub: this feature is so core to GitHub that there is no option to disable it! 
> 
> The current proposal is to allow to integrate contributions to the LLVM project directly from pull-requests. In particular the exact setup would be the following:
> 
>   - Contributors should use their own fork and push a branch in their fork.
>   - Reviews can be performed on GitHub. The canonical tools are still the mailing-list and Phabricator: a reviewer can request the review to move to Phabricator.
>   - The only option available will be to “squash and merge”. This mode of review matches the closest our current workflow (both phabricator and mailing-list): conceptually independent contributions belongs to separate review threads, and thus separate pull-requests. 
> This also allow the round of reviews to not force-push the original branch and accumulate commits: this keeps the contextual history of comments and allow to visualize the incremental diff across revision of the pull-request. 
>   - Upon “merge” of a pull-request: history is linear and a single commit lands in master after review is completed.
> 
> As an alternative staging proposal: we could enable pull-requests only for a small subset of sub-projects in LLVM (i.e. not LLVM/clang to begin with for example) in the repo. In this case, we would propose to begin with the MLIR project (as soon as it gets integrated in the monorepo). This would be a good candidate to be the guinea pig for this process since it does not yet have a wide established community of contributors, and the current contributors are already exclusively using pull-requests.
> 
> Here is a more complete doc on the topic: https://docs.google.com/document/d/1DSHQrfydSjoqU9zEnj3rIcds6YN59Jxc37MdiggOyaI
> 
> Cheers,
> 
> -- 
> Mehdi
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

--
Sam Elliott
Software Developer - LLVM
lowRISC CIC
--



More information about the llvm-dev mailing list