[PATCH] RFC: CONTRIBUTING: Embrace gitlab

[Daniel Stone]

Hi,

On Thu, 13 Sep 2018 at 09:54, Jani Nikula <jani.nikula at intel.com> wrote:
> On Wed, 12 Sep 2018, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > On Wed, Sep 12, 2018 at 7:50 PM, Sean Paul <sean at poorly.run> wrote:
> >> Given the size of this repo and the number of contributors, I'm not
> >> convinced either of these should be blockers. We should avoid merge
> >> commits since the volume is low enough that having to rebase should
> >> be quite rare. Reviews and acks will be recorded in the merge request,
> >> which is easily accessed from the UI.
> >
> > Not sure recording review&acks on the merge request is going to fly
> > for kernel. And that's kinda what I want to test-drive here, in 1st
> > gear :-)
>
> If the acks and reviews aren't recorded in git log for each commit, they
> don't exist.

To be fair, given that DRM is about the only subsystem doing it, does
recording it in a fixed format in the commit matter? ;)

> For submitting patches and merging them I can live with having to point
> and click on a web site.
>
> Both of those seem like something that can be automated too.

Yeah, they can. The API is pretty easy to drive, and you can create
MRs through it, e.g.
https://python-gitlab.readthedocs.io/en/stable/gl_objects/mrs.html#id2

There are also clients in other clients if you prefer, or it's just
REST so you can always just drive curl + jq yourself.

> But by far the biggest change that is being proposed here between the
> lines is moving code review to a gitlab web site, in a one-size-fits-all
> model. I could no longer choose and use the tools I prefer for code
> review. Tools that I can and have customized for my needs and
> preferences. I could no longer review code using the same tools I use to
> read and write code (as well as email, and pretty much everything
> else). That would be a severe performance hit for me.
>
> Perhaps I could tolerate that for maintainer-tools, but I won't endorse
> gitlab based reviews for any kernel work.
>
> With that, I think the question should be how we adapt gitlab to our
> needs, not how we adapt our needs to gitlab. How do we preserve email
> based review while trying to get as much of the benefits of gitlab as
> possible? And shouldn't the trials of (at least some of the) smaller
> projects such as maintainer-tools be taking that into account, instead
> of test driving something that won't fly in kernel?

In Weston we ended up deciding the web UI was a better flow despite
its obvious issues, and that the web/mail review models had too high
an impedance mismatch to try to combine, though YMMV. We did think
pretty hard about how to preserve R-b/A-b tags i ncommits since we
liked them, but we ended up concluding that since we placed the tags
manually anyway (Patchwork isn't good at the nuance of 'with XYZ
fixed, this is R-b', etc), and didn't use them for much of value, we
were better off using the full range of the English language, rather
than trying to reduce our entire review process down to one or two
tags which are quite cumbersome to type out anyway. Though
https://gitlab.com/gitlab-org/gitlab-ce/issues/47520 does exist and
that's something I could help push if there was concrete demand for
it.

Patchwork was a pretty cautionary tale for inferring meaning from
mail, and even then all it ever tried to do is know what a patch was,
what an email reply was, and store them away. But mapping between
GitLab and Patchwork is harder:
  - it still has the same inbuilt series/commit definitions, though
being real Git trees these have actual SHAs
  - like free text in mail replies, you can attach comments to a MR
  - somewhat like mail threading, you can 'start a discussion' in a
MR, which takes discussions out into a thread with a persistent ID,
also having an explicit resolved/unresolved state
  - you can either create comments or resolvable discussions on
specific lines of a commit as well, which are preserved as well as
possible between series revisions

Mapping these concepts from GitLab to email is pretty easy, and
multiple people (similarly rusted-on command-line people who have
strong opinions on mail headers) have commented they're impressed how
well 'threading' in the web UI maps to mail headers and message trees
in mutt/notmuch. Mapping from email back to that is ... harder. It
would require Patchwork to interpret the context of the mail (and how
everyone's different MUAs handle quoting, including when you nest a
bit too deeply and wrapping kicks in, woohoo) to find out where to
place the comments.

I think it would be far more promising to write native Emacs
integration using the API directly, than trying to derive meaning from
free-form mail. Maybe I'm too pessimistic, but I just don't see the
latter being good enough to climb out of the uncanny valley for
two-way integration. And getting it wrong is just going to breed
resentment from both sets of users. I can't offer to help with the
Lisp side of things (I have vi mode in my shell and grey hairs in my
beard, there's no changing now), but I'm happy to help either with
using the GitLab API or proxying feature requests upstream, if that's
something you wanted to take on. Or failing that, even using something
like git-notes seems like a less-lossy proxy than mail, where we still
can't get the basics right after 26 years of SMTP.

Anyway, I wrote https://gitlab.freedesktop.org/snippets/503 to at
least scrape the non-diff comments from GitLab, so you could start
scraping like Patchwork. You can use the MR versions API in the exact
same way as the notes API to get a list of revisions and when they
were posted, if you wanted to attempt to match R-b/A-b comment tags to
revision history.

Cheers,
Daniel