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