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

Thu Nov 7 00:39:13 PST 2019

Hi Mehdi,

I support the idea of running some experiments to gain more insights.

If you're interested: I can offer to add the pre-merge tests to the github
pull-requests as well. This way we can also see what the CI integration in
github would look like.

On Thu, Nov 7, 2019 at 9:09 AM Roman Lebedev via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Strong -1 personally.
>
> * What is the endgoal? To fully kill phab and move to github pullrequests?
>   it might be best to discuss *that* first. (did i miss an RFC?)
> * Separation of attention - does everyone who cares
>   now has to also look at pull requests for reviews;
>   or should they be exempt from general review attention?
> * For me personally after using phabricator, github now seems
>   extremely crude, laggy, limited. To name a few:
>   * There is no way to see previous version of the patch.
>     I don't think there is any way to disable force-push for PR's.
>     While this is only 'slightly' limiting for the reviewer,
>     this can be more limiting for the author - how do i go back
>     to previous revision? I can't, i need to maintain a copy
>     of every branch i pushed manually.
>   * Impossible to chain reviews - a PR diff can only be made
>     on top of git master branch. Any non-trivial change consists of
>     separable PR's. Now either one will only be able to submit
>     dependent PR after the prereqs are merged, or the diff will be
>     impossible to read.
>   * Does not load large diffs by default.
>     That caught me by surprise several times
>     when i was searching for something.
>   * No diffs in mail - *super* inconvenient.
>     One has to open each pr in browser (or fetch via git etc etc)
>   * Github is an official US-based commercial organisation.
>     It *has* to follow U.S. export law. In particular i'm thinking of
>       https://techcrunch.com/2019/07/29/github-ban-sanctioned-countries/
>       https://github.com/tkashkin/GameHub/issues/289
>     Does phabricator already have such restrictions, blocks?
>     If not, wouldn't you say adding such restrictions is not being
>     more open for contributors?
>     What happens when, in not so long in the future, the entirety of, say,
>     china or russian federation is blocked as such?
>   * Same question can be asked about internet "iron" curtains
>     certain (*cough*) countries are raising. That also has already happened
>     before (and *will* happen again), and i was personally affected:
>       https://en.wikipedia.org/wiki/Censorship_of_GitHub#Russia
>     I don't recall that happening to phabricator yet.
>     I fail to see how that is more contributor-friendly.
>   * Not sure anyone cares, but while using github as main git
>     repository "mirror" is perfectly fine - git is distributed, only
> canonical
>     write-repo would be affected anything bad happen. But that isn't the
> case
>     for reviews, issues; as it has been discussed in the "let's migrate
> bugzilla
>     to github issues", it is far more involved.
>   * What about DMCA? Not sure how this is currently handled.
>   * UI feels laggy. Not much to add here, pretty subjective.
>   * I'm sure i'm missing a few.
>
> The github does come with it's benefits, sure:
> * It is *simpler* to preserve git commit author.
>   Currently one has to ask the author for the "Author: e at ma.il" line,
>   and do `git commit --amend --author="<>"`.
> * @mention is wide-r-reaching - whole github, not just llvm phabricator
> * No more "phabricator disk full" issues
> * ???
>
> TLDR: such migration lowers the bar for new, first time,
> unestabilished contributors, but i personally feel it *significantly*
> raises the bar for the existing contributors, reviewers.
> We don't live in perfect world. Aspirational goals are aspirational.
> They should be attempted to be reached, but they shouldn't shadow and
> overweight, take over the main goal of the LLVM project.
>
> Personally, i don't see that benefits out-/over- weight the drawbacks.
>
> Roman.
>
> On Thu, Nov 7, 2019 at 8: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
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>

-- 
Best,
Christian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20191107/9d71d91f/attachment.html>