Re: patch submission process, was Re: [PATCH v6 06/16] merge_recursive: abort properly upon errors

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Aug 4, 2016, 15:58 UTC
Message-ID: <alpine.DEB.2.20.1608041730130.5786@virtualbox>
In-Reply-To: <CAGZ79kYWdZCNW_eBi5aLAacyBZJXQ9xyOWMBmjNsYT5NWjr-Og@mail.gmail.com>

Hi Stefan,

On Wed, 3 Aug 2016, Stefan Beller wrote:

Show 30 quoted lines

> On Wed, Aug 3, 2016 at 9:07 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > On Wed, 3 Aug 2016, Junio C Hamano wrote:
> >
> >> On Wed, Aug 3, 2016 at 4:59 AM, Johannes Schindelin
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> >
> >> > I disagree, however, with the suggestion to sift through your `pu`
> >> > branch and to somehow replace local branches with the commits found
> >> > there.
> >>
> >> To be more in line with the "e-mailed patch" workflow, I think what I
> >> should do is to send the version I queued with fixups back to the
> >> list as follow-up.  Just like reviewers review, the maintainer
> >> reviews and queues, the original author should be able to work in the
> >> same workflow, i.e. reading and applying an improved version of the
> >> patch from her mailbox.
> >
> > You seem to assume that it isn't cumbersome for people like me to
> > extract patches out of mails and to replace existing commits using
> > those patches.
> >
> > So it probably comes as a huge surprise to you to learn that this *is*
> > cumbersome for me.
> 
> It is also cumbersome for me, because I never had the need to setup a
> proper mail client that has the strength to apply patches. The need was
> not there as I tend to apply only rarely patches by email, so I can go
> the painful way each time.

The reason is clear, too. Mail clients serve humans. That is their purpose. Humans do not care all that much whether the text was preserved exactly as the sender wrote it, except rich text (read: HTML), of course.

Show 23 quoted lines

> > I got too used to the ease of git push, git pull with or without
> > --rebase, and many other Git commands. Having to transmogrify code
> > changes from commits in Git into a completely different universe:
> > plain text patches in my mailbox, and back, losing all kinds of data
> > in the process, is just not at all that easy. And it costs a lot of
> > time.
> >
> > In short: if you start "submitting patches" back to me via mail, it
> > does not help me. It makes things harder for me. In particular when
> > you add your sign-off to every patch and I have to strip it.
> 
> You don't have to strip the sign off, as it shows the flow of the patch,
> e.g.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> may indicate you proposed a patch, Junio picked it up (and fixed a typo
> optionally), I obtained the patch (via mail, via Git?) improved it, you
> improved it further and then Junio took it and merged it upstream.

Recently, I got yelled at because I took one of Junio's patches, made a couple of changes, and *added* my sign-off.

Before that incident, I agreed with you that it may make for a nice record of the back-and-forth that eventually resulted in the patch in question. Now, I am not so sure anymore.

Show 8 quoted lines

> > If you change your workflow, I would humbly request that you do it in
> > a way that makes things easier on both sides, not harder.
> 
> When attending the Git Merge conference in May, gregkh said roughtly:
> "We deliberately waste developers time, because it is not scarce.
> Maintainers time is scarce however " and it stuck with me. (and I am a
> developer, not a maintainer ;( so at least the kernel community deems it
> ok to waste my time).

Yeah. It was not the only thing I disagreed with in his talk. To be a little bit blunt (by my standards, not by LKML's standards, that is): the Linux kernel mailing list is not necessarily anything I would want to use as a role model.

I agree that maintainers' time is scarce.

I am one.

So of course I agree with that statement. What I disagree with is that it is okay to *waste* contributors' time. That's just inconsiderate. And I say that also because I am a contributor *in addition* to being a maintainer.

As a consequence, I commend Greg for recognizing that the patch submission process must be light on the maintainer. And I would have commended him even further if he had realized that proper tooling should waste nothing, and no one's time.

Show 6 quoted lines

> While that is true for the kernel community, I guess it is also true for
> the Git community, unless Junio (and the community) want to appoint a
> bunch of maintainer lieutenants, such that they outnumber the number of
> developers, e.g. divided by areas of the code: a refs backend
> maintainer, a submodule maintainer, ...  or rather by area of usage: a
> porcelain UI maintainer, a git-on-server maintainer.

As I mentioned earlier, I do not care much about following LKML's example.

What I see on this here list is that many a potential contributor is scared away, that we waste precious time (also the maintainer's) pointing out in what way certain contributions do not follow the guide lines, and that even old-timers sometimes submit patches that are white-space corrupted.

That is a *huge* waste of time. In my opinion, the culprit is that we do not use appropriate tools. To a mail client, everything looks like a nail. Wrong metaphor, but you get the point.

Show 15 quoted lines

> > It would be a totally different matter, of course, if you used the
> > branches I publish via my GitHub repository, added fixup! and squash!
> > commits, published the result to a public repository and then told me
> > to pull from there, that would make things easier. We could even
> > introduce a reword! construct, to make the review of the suggested
> > edits of the commit message easier. I could easily verify that my
> > branch head agrees with the base commit of your branch, I could build
> > proper tooling around this workflow, and it would lighten my load.
> >
> > I guess what I am saying is that we might just as well start using this
> > awesome tool to work with code, that tool named "Git".
> 
> I think Git itself is for the tracking the code and managing it, e.g.
> merging, moving, keeping it. That doesn't quite include modifying and
> creating code (e.g. there is no "git edit" command)

Git is not only for tracking the code. It knows about editors (core.editor), it can export .zip files (git-archive), it can show human-readable (not machine-readable) word diffs, etc.

Whenever we needed a certain functionality, we added it.

> If we were to change our workflows drastically, I'd propose to
> go a way[1] similar to notedb in Gerrit, or git-series,

Gerrit is a huge, non-distributed system. Complex, too. If we change the patch submission process, we should make things *easier*, not *harder*. So I think Gerrit is pretty much out of the question.

Even requiring every contributor to register with GitHub would be too much of a limitation, I would wager.

And when I said I have zero interest in tools that use the "latest and greatest language", I was hinting at git-series. Rust may be a fine and wonderful language. Implementing git-series in Rust, however, immediately limited the potential engagement with developers dramatically.

Additionally, I would like to point out that defining a way to store reviews in Git is not necessarily improving the way our code contribution process works. If you want to record the discussions revolving around the code, I think public-inbox already does a pretty good job at that.

I guess I have no really good idea yet, either, how to retain the ease of access of sending mails to the list, yet somehow keep a strong tie with the original data stored in Git.

Ciao, Dscho