[libvirt] [RFC] Accepting PRs/MRs for libvirt on GitHub/GitLab

[Daniel Henrique Barboza]

On 10/16/19 9:22 AM, Andrea Bolognani wrote:
> As we look to make the libvirt project easier to contribute to, one
> fact that certainly comes to mind is that we only accept patches via
> the mailing list. While the core developers are comfortable with the
> email-based workflow and swear by it, many perspective contributors
> are used to the PR/MR workflow common to many Open Source projects
> these days, and similarly swear by it.
>
> If we look at the PRs submitted on GitHub against the libvirt mirror
> repository[1], there's just three of them per year on average, which
> is arguably not a lot; however, it should be noted that each
> repository also carries a fairly loud "PULL REQUESTS ARE IGNORED"
> message right at the top, and it's not unlikely that a number of
> perspective contributors simply lost interest after seeing it.
>
> As a way to make contributions easier without forcing core developers
> to give up their current workflow or making the project dependent on
> a third-party provider, I suggest we adopt a hybrid approach.
>
> First of all, we'd remove the ominous message from GitHub mirror
> repositories (interestingly, the same is not present on GitLab).
>
> Then, we'd starting accepting PRs/MRs. The way we'd handle them is
> that, when one comes in, one among the handful of core developers who
> volunteered to do so would review the patches on the respective
> platform, working with the submitter to get it into shape just like
> they would do on the mailing list; once the series is finally ready
> to be merged, the core developer would update the PR/MR as necessary,
> for example picking up R-bs or fixing the kind of trivial issues that
> usually don't warrant a repost, and then push to master as usual.

This would mean that there will be patches that will get accepted
without the ML being aware of. The core developer (or the author
itself) would need to send the revised patches to the ML before
pushing to make people aware of it before pushing to master, IMO.

I have a 2 year experience maintaining a now defunct project in
Github (a KVM web management tool called Kimchi) in which we used a
hybrid approach like I think you're suggesting, with mailing list, people
opening bugs in Github + Github PRs. It was annoying - people will 
inevitably
discuss issues using the Github tracking system, which means that you'll
have to subscribe via email to Github notifications if you didn't want 
to miss
out. It was common for people that used the ML to start discussions that
were already happening in the Web, and vice-versa.

All that to make a case against Libvirt having additional ways of
communication. I don't mind pull requests from Github/Gitlab as long as the
ML is made aware of, before the patches are accepted. But people bringing
up discussions in the ML and Github/Gitlab scales poorly, in my experience.


DHB


>
> More in detail: GitHub and GitLab have a feature that allows project
> members to update PRs/MRs: basically the way it works is that, if the
> PR/MR was created by the user 'foo' from their branch 'bar', the
> libvirt developer is allowed to (force-)push to the foo/libvirt/bar
> branch, and doing so updates the corresponding PR/MR; after that, if
> the updated branch is locally merged into master and master is pushed
> to the libvirt.org repo, once it gets mirrored GitHub/GitLab will
> recognize that the commit IDs match and automatically mark the PR/MR
> as merged. I have tested this behavior on both platforms (minus the
> mirroring part) with Martin's help.
>
> One last important bit. In the spirit of not requiring core
> developers to alter their workflow, the developer who reviewed the
> patches on GitHub/GitLab will also be responsible to format them to
> the mailing list after merging them: this way, even those who don't
> have a GitHub/GitLab account will get a chance to take notice of the
> code changes and weigh in. Unlike what we're used to, this feedback
> will come after the fact, but assuming that issues are spotted only
> at that point we can either push the relevant fixes as follow-up
> commits or even revert the series outright, so I don't feel like it
> would be a massive problem overall.
>
> Thoughts?
>
>
> [1] https://github.com/libvirt/libvirt/pulls?q=is:pr+is:closed