Clarification about "Better tooling for reviews", was Re: Google Doc about the Contributors' Summit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

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

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]