Re: Widening reach for ... code?


Andrew Grimberg
 

On 2020-08-24 14:35, Robert Varga wrote:
On 24/08/2020 22:59, Andrew Grimberg wrote:
How about LF(N) investing some dev/test resources into making it
possible for outside contributors to be mirrored back to Gerrit?
Technically we can do this already. There is a GitHub plugin for Gerrit
that will allow us to create Gerrit side changes if a PR is opened a
GitHub repository. That being said, I've played with this workflow over
on GerritHub and it seriously hurts.

The workflow operates in the following manner:

* PR is created in GitHub

* For every commit in the PR a new Gerrit change is created
So far so good.

* If changes are requested on the PR then the modified code ends up
coming in as a _separate_ Gerrit Change. The requested changes are _not_
communicated back to the open PR, they happen internal to the Gerrit
system. If the system is configured to not send mail to unknown entities
(as all of ours are) then the person that raised the PR, if they have
never logged into the Gerrit system, will _not_ get notified.
Okay, so this is where LF(N) or ODL budgeting gets involved. The plugin
needs to be improved to mirror any comments back to the PR. IIRC at
least GH has APIs to do this.
There's an additional issue with that. In the Gerrit workflow you amend
your change, effectively creating a brand new commit object (different
commit sha) which _replaces_ the previous patch in your change series.

Doing something with a PR breaks down because by default a PR is a
sequence of changes that are completely chronological, so a fix in it
means you end up with a new commit object with just that change and it
gets added to the end of the PR commit chain.

To properly work with Gerrit in the work flow would require folks to
force push their fixes back to their repo destroying (or at least
orphaning) the old versions of their prior commits in the chain.

Now, if we can get the PR -> Gerrit thing working correctly, that may
not be too much of an issue as the prior patches would still live in the
Gerrit change history but getting people on the GitHub side to do things
cleanly is going to be difficult.

I suppose Ideally what would happen would be that if a change was
requested then the PR would be closed and the contributor would have to
open a new PR, but if they don't have the same Change-ID (which we
_might_ be able to force into their previous commit message) it's still
going to disassociate and become a different Gerrit Change.

* Once the change is merged on the Gerrit side, the code gets mirrored
back to GitHub but the PR does not get closed.
Same as above, this is just being plain stupid. Except for your final
point...

Additionally, and I don't know if this is just a limitation of how
GerritHub operates, the PRs don't automatically create Gerrit Changes.
You have to explicitly force a PR refresh cycle.

Now, some of this is obviously just a failing in how the plugin
operates, but the biggest issue here is that the GitHub/GitLab workflow
is very much an anti-pattern in Gerrit. Gerrit requires that Change-ID
to be able to track different patches across the life of a single
change. You don't get that in GitHub/GitLab, you just got a very messy
patch series to contend with.
... right, which is where our pre-commit hooks et al. get into play.

A very obvious solution is to:

1) require all PRs to have 'Allow edits by maintainers' enabled, which
is the default at GH at least.

2) have a bot that updates all PR patches to include Change-ID (yeah,
the bot has to be a maintainer, but LF should have zero problems running
at that privilege level)

3) that effectively means we have bridged the PR/patch gap, hence
re-pushes end up correlating correctly -- i.e. the bot knows the PR
number and patch (seqnr|headline|...) hence can (re-)assign correct
change-IDs and/or issue warnings/whatever. Yes, it is not plain GH/GL
workflow, but hey, I'll bet anyone a (non-sour!) beer it can be made to
work 95% of time.

But that circles back to allocating budget (== people) to taking this
analysis to a solution. It seems we have a fruitful exploration here and
nothing says 'utterly unfeasible' so far -- perhaps we need an RFP or
similar?

Regards,
Robert
--
Andrew J Grimberg
Manager Release Engineering
The Linux Foundation

Join {TSC@lists.opendaylight.org to automatically receive all group messages.