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

Daniel P. Berrangé berrange at redhat.com
Wed Oct 16 16:08:20 UTC 2019


On Wed, Oct 16, 2019 at 05:03:31PM +0200, Ján Tomko wrote:
> On Wed, Oct 16, 2019 at 02:22:56PM +0200, Andrea Bolognani wrote:
> > First of all, we'd remove the ominous message from GitHub mirror
> > repositories (interestingly, the same is not present on GitLab).
> > 
> 
> Well if you're using GitLab then you're already aware of the fact that
> not everything is hosted on GitHub.
> 
> > 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.
> > 
> > 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.
> > 
> 
> Here it deviates from the usual mailing list workflow where the patch
> has (in theory) a chance to be seen by all the developers.
> 
> But given that the requests will probably
> a) be close to trivial
> b) seen by a group of developers, not just one

I wouldn't expect the changes to be trivial. Current stuff
is trivial largely because we tell people not to open merge
requests. If we adopt use of web based review, then expect
people to submit non-trivial patches. I would do so myself
for example. Thus I think we must make a clean switchover
from email to a single web based tool.
 
> sending it to the mailing list after it's pushed is better treatment
> than our language bindings get.

The situation with our language bindings has never felt very
comfortable to me. Except for Python, they rarely get posted
for review. Using a web based tool merge requests for language
bindings would be a step forward for transparency in what
we're submitting for them. If nothing else, it will ensure
we can wire up CI to run gating pushes to master to avoid
broken builds.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list