git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Clarification about "Better tooling for reviews", was Re: Google Doc about the Contributors' Summit
Date: Wed, 15 Feb 2017 13:02:29 +0100 (CET)
Message-ID: <alpine.DEB.2.20.1702151215570.3496@virtualbox> (raw)
In-Reply-To: <xmqq4m01if1z.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Fri, 10 Feb 2017, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Technically, it is not a write-up, and I never meant it to be that. I
> > intended this document to help me remember what had been discussed,
> > and I doubt it is useful at all to anybody who has not been there.
> >
> > I abused the Git mailing list to share that link, what I really should
> > have done is to use an URL shortener and jot the result down on the
> > whiteboard.
> >
> > Very sorry for that,
> 
> Heh, no need to apologize.

Well, you clearly misunderstood the purpose of the document, and that was
my fault, as I had not made that clear.

> I saw your <alpine.DEB.2.20.1702071248430.3496@virtualbox> that was
> sent to the list long after the event, which obviously no longer
> meant for collaborative note taking and thought that you are
> inviting others to read the result of that note taking, and that is
> why I commented on that.

Of course you are free to read it, and to guess from the sparse notes
around what the discussions revolved. I do not think that you'll get much
out of the notes, though.

> I've hopefully touched some "ask Junio what he thinks of this" items and
> the whole thing was not wasted ;-)

I am afraid that it was quite clear to everybody in the room "what you
think of this".

And I do not think that your clarifications how you review code had any
direct relation with the discussions in particular about better tooling
for the review process.

To open the discussion of that particular part of the Contributors'
Summit, I mentioned a couple of pain points, and the remainder of the
discussion really revolved around those, constructively so, I want to add:

- we actively keep potential contributors out by insisting that email
  communication is the most open and inclusive, when right off the bat,
  without any notice to anybody, our mailing list rejects mails sent both
  by the most popular desktop mail client as well as by the most popular
  web mail client.

- developers who really, really want to contribute may switch email
  clients or try to configure their existing email clients to skip the
  HTML part of their emails, only to be met with the reply "your patch is
  white-space corrupted" which cannot fail to sound harsh.

- the few contributors not deterred by this problem, and persistent enough
  to try until they manage to get through, often drop the ball after being
  met with suggestions that would ideally be addressed by automation so
  that humans can focus on problems only humans can solve (every time I
  read "this line is too long" in a review I want to cry: how is this
  worth the time of contributor or reviewer? There are *tools* for that).

- discussions often veer into completely tangential topics so that the
  actual review of the patches is drowned out (and subsequently
  forgotten).

- for any given patch series, it requires a good amount of manual digging
  to figure out what its current state is: what reviewers' comments are
  still unaddressed? Is the patch series in pu/next/master yet? Is the
  *correct* iteration of the patch series in pu/next/master? How does the
  version in pu/next/master differ from what the contributor has in their
  local repository? etc

- the closest thing to "this is the state of that patch series" is the
  What's Cooking email that neither CC:s the original contributors nor
  does it bear any reliable link to the original patch mail, let alone the
  original commit(s) in the contributor's repository.

I really do not think that any of your descriptions of your workflow and
of your review priorities could possibly be expected to fix any of these.

I have an additional pain point, of course, in that your priorities in
patch review (let's admit that it is not a code review, but a patch review
instead, and as such limited in other ways than just the lack of focus on
avoiding potential bugs) are unfortunate in my opinion. But that is not
your problem.

It is clear to me that these pain points only affect potential
contributors (and some of them only frequent contributors who are as
uncomfortable with the sheer amount of tedious "where is that mail that
contained that patch? Oh, and what was the latest reply to this one? Okay,
and in which worktree do I have those changes again?" type of things that
really should not by *my* burden, given that we are trying to develop an
application that helps relieve developers of tedious burden by automating
recurring tasks).

That is why I did not call session "Let's criticize Junio for something
that does not even bother him", but instead "Better tooling for reviews".

The only way forward, as I see it, is for other like-minded contributors
(one of the GitMerge talks even dedicated a good chunk of time to the
description of the pitfalls of the Git mailing list, and home-grown tools
to work around them, so I am definitely not alone) to come together and
try to consolidate and develop the tools to work around the problems
incurred from the choice of using the Git mailing list as the only vehicle
for code contributions [*1*].

When (or if?) those tools become polished and convenient enough to make a
difference, who knows? Maybe even you or Peff find them useful enough to
switch. After all, even The Linus switched from tarballs and patches to
BitKeeper (and I read that it took only a substantial amount of time and
effort and money on Larry McVoy's part[*2*]).

But nothing of that comes as news to you. You knew all that about my
opinions and my pains. I actually did not expect there to be any benefit
in the "Better tools for review" session for you, specifically, and it
seems I was correct in that assumption.

Ciao,
Johannes

Footnote *1*: git-svn, gitk and git-gui Pull Requests and subsequent
merges do not count, as it is clear that they are not *really* part of the
Git source code, and therefore not *really* reviewed on the Git mailing
list.

Footnote *2*: Some posts I read about it are unfortuntely gone, but here
is an archived one that still works:
https://web.archive.org/web/20120414183625/http://kerneltrap.org/node/222

      reply index

Thread overview: 6+ messages in thread (expand / mbox.gz / Atom feed / [top])
2017-02-02  9:08 Johannes Schindelin
2017-02-07 11:51 ` Johannes Schindelin
2017-02-09 19:09 ` Junio C Hamano
2017-02-10 15:59   ` Johannes Schindelin
2017-02-11  3:26     ` Junio C Hamano
2017-02-15 12:02       ` Johannes Schindelin [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply to all the recipients using the --to, --cc,
  and --in-reply-to switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1702151215570.3496@virtualbox \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox