[llvm-dev] Allowing PRs on GitHub for some subprojects

Thu Mar 5 09:53:08 PST 2020

On Wed, Mar 4, 2020 at 12:13 PM Mehdi AMINI <joker.eph at gmail.com> wrote:

>
>
> On Wed, Mar 4, 2020 at 8:14 AM Louis Dionne <ldionne at apple.com> wrote:
>
>> Mehdi, Chris & others,
>>
>> I guess I did not express the main reasons for wanting to switch over
>> very well in my original message.
>>
>
> You original message was about “ commit attribution”, but now it is all
> about testing?
>
> Instead of jumping to a solution (pull-request) why not expressing the
> actual problem (lack of pre merge testing) and discuss it and explore all
> the possible solutions?
> I think the discussion would be much more productive if we take it from
> first principles here.
>

The first principle I would like to work from is "Keeping libc++ bugfree
while enabling development velocity.".
Build-policing, maintaining infrastructure, and keeping haunted graveyards
are jobs I wish to give up.

>
>
> Like Christian talked about, for me it's all about pre-commit testing. I
>> believe pre-commit testing is a widely shared desire among this community.
>> However, how badly it is missed depends on sub-projects, because they have
>> different realities. For example, in libc++:
>>
>> 1. We have a lot of first-time contributors, which means that the
>> maintainers end up shepherding many contributions. In particular, this
>> often means fixing small breakage following their changes, which can be
>> difficult for them because they can't reproduce the failures locally, and
>> they might not even know where to look. While these contributors can submit
>> valuable improvements and bug fixes, we can't expect them to fix every last
>> platform that we support in the current state of things -- it's hard, it's
>> boring, and it's stressful.
>>
>> 2. Our testing matrix is very large, and interactions between different
>> configurations (usually #ifs/#elses) is very subtle. This means the rate of
>> mistake-on-first-try is, I think, higher in libc++ than in most other LLVM
>> projects. Even with careful review, I find that a large percentage of
>> changes end up breaking something somewhere, and I have to fix it (usually
>> quickly enough to avoid reverting).
>>
>> As a result, the lack of pre-commit testing is actively harming the
>> health of libc++ as a project. It might be true for other projects as well,
>> but I can only speak for libc++ because that's where I have first hand
>> experience.
>>
>
> What changed recently that makes this suddenly critical compared to the
> previous years?
>
> The number of new contributors, and the amount of development being done
by other groups.

>
>
> Unfortunately, we currently don't have a good way of doing pre-commit
>> testing on Phabricator AFAICT
>>
>
>
> I thought we do now? I got a bunch of libcxx failing on my revision a few
> weeks ago.
> The LLVM-premerge-test project recently added presubmit on Linux.
> Windows will hopefully follow soon (in beta right now I believe), and Mac
> afterward!  (Even though mac is lacking on the cloud provider availability)
>
>
> . From the Harbormaster documentation [1]:
>>
>>     You'll need to write a nontrivial amount of code to get this working
>> today. In the future, Harbormaster will become more powerful and have more
>> builtin support for interacting with build systems.
>>
>> So while I appreciate all the efforts being made in this area, I still
>> don't even know where to start if I want to setup pre-commit testing for
>> libc++ today. However, the path is very clear with GitHub PRs and there are
>> many options available.
>>
>> Whenever I hear arguments of dividing the community, not being able to
>> share infrastructure, the lack of Herald -- those all make a lot of sense
>> to me and I think they're good arguments.
>>
> However, it is clear that folks who even think about these arguments are
>> not paying the same cost for the lack of good pre-commit testing that I'm
>> paying on a weekly basis,
>>
>
> FYI that can read quite condescending...
>
>
> because for me that outweighs everything else.
>>
>
>> I don't know how to come to a decision here, all I know is that libc++
>> needs to get out of the status quo soon. And if the solution is that
>> Harbormaster suddenly becomes usable without an unreasonable time
>> investment from me, then I'm fine with that too. I'm not looking to switch
>> to GitHub PRs for the sake of it, I'm looking to solve problems that are
>> harming libc++ in the current system.
>>
>> Cheers,
>> Louis
>>
>> [1]:
>> https://secure.phabricator.com/book/phabricator/article/harbormaster/
>>
>>
>> On Feb 29, 2020, at 23:06, Mehdi AMINI <joker.eph at gmail.com> wrote:
>>
>>
>>
>> On Tue, Feb 25, 2020 at 4:19 AM Christian Kühnel via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Hi Louis,
>>>
>>> I think this is a good idea. We should start with some local experiments
>>> where people are willing to try it and figure out how well that works and
>>> what does not. Why not allow this for "not significant" changes? They are
>>> merged without review today, so we could do them with reviews (and
>>> automated tests) via pull requests instead.
>>>
>>
>> I still feel this is only a recipe for confusion if "some" pull-requests
>> are accepted on Github but not all. So -1 from me on this.
>>
>>
>>>
>>> @Mehdi
>>>
>>>> - it does not favor to build common tooling: the recent work on
>>>> enabling pre-submit CI tests on Phabricator is valuable and I'm looking
>>>> forward to get this extended. But splitting the various ways of
>>>> contributing to the repo just means more infrastructure to build to sustain
>>>> this kind of efforts. (the infrastructure is easier built on GitHub by
>>>> the way, but that is an argument in favor of migrating from Phab to GH for
>>>> the full-project).
>>>>
>>>
>>> Oh I'm happy to add Github support as soon as someone switches on PRs.
>>> This is soooooo much easier to set up and maintain than the Phabricator
>>> integration. And we already have builds for the release branch (
>>> https://buildkite.com/llvm-project/llvm-release-builds) anyway. So we
>>> could easily scale that up. And we can only get pre-merge testing on
>>> Phabricator to a certain point, as it's not triggering builds for ~50% of
>>> the code reviews.
>>>
>>> @Chris Lattner
>>>
>>>> Although I am one of the (many) people who would love to see us move
>>>> from Phabricator to GitHub PRs, I think it is super important that we
>>>> do the transition all at once to keep the LLVM community together.  I’m
>>>> already concerned about the fragmentation the discourse server is causing,
>>>> e.g. MLIR not using a -dev list.  I’d rather the community processes stay
>>>> consistent.
>>>>
>>>
>>> Please allow me to disagree there. IMHO we're way too large and diverse
>>> of a project to do binary, overnight transitions.
>>>
>>
>> You seem to be arguing the "how to transition" while there is no
>> agreement on a transition happening in the first place.
>>
>>
>>> We're also too large to follow a one-size-fits-all approach. If we agree,
>>>
>>
>> I don't: we went with a monorepo because we believed that the
>> one-size-fits-all would be more beneficial than splitting, both in terms of
>> infrastructure, but also in terms of the practices of the community, etc.
>>
>> Github PRs are the right glow, why take this step-by-step. We should have
>>> something like a list of important and supported use cases/interactions for
>>> the infrastructure. Then we could start working on them one-by-one and
>>> figure out if/how they could be implemented on Github and how we could do a
>>> smooth transition between these.
>>>
>>> If Herald rules are important: Find a way to implement something similar
>>> for Github. Maybe there is even a market for such a tool.
>>> If transparency is the problem: Find a way to mirror PRs into
>>> Phabricator, so people can at least see them there.
>>> We're not restricted to community contributions there. We can also pay
>>> someone to build the things we need.
>>>
>>
>> One aspect here though is that we can pay someone to build the things we
>> need in Phabricator, we can't change GitHub though.
>> It was mentioned in the past that we should engage with GitHub and see if
>> they would add the feature we're missing to their roadmap, if it hasn't
>> been done I'd start there: building up this list of things that need to
>> happens before we can agree towards a transition, and engaging with GitHub
>> to have these.
>>
>> --
>> Mehdi
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200305/9a5f9580/attachment.html>