Pain points in Git's patch flow

[Atharva Raykar]

On 14-Apr-2021, at 11:43, Jonathan Nieder <jrnieder at gmail.com> wrote:
> 
> Hi,
> 
> I'd like to introduce Raxel (cc-ed), who is starting an internship
> this June with the Git team at Google.
> 
> He'll be working on a bit of an experimental project: we want to take
> Patchwork[1], which in principle can be a helpful addition to a
> mailing list centric workflow[2], and improve it to be something that
> people in the Git open source project get day-to-day benefit from.
> Raxel's previous successes in making changes to tools to support a
> better user experience make me excited for the potential for this
> work.
> 
> Anyway, yesterday[3] Junio, Taylor, and Emily were discussing how to
> encourage more reviews:
> 
> <gitster> this week, i'd be thinking about ways to get topics, that
>           are not reviewed sufficiently, reviewed. I can act as the
>           last-resort fallback reviewer, but that's not sufficient.
> <ttaylorr> gitster: I share your concern.
> <nasamuffin> gitster: yep, agree, on both counts
> 
> That reminded me that it would be useful preparation to collect
> descriptions of pain points we are having with our existing patch
> flow.  For example:
> 
> - As a reviewer, I want to be able to easily find a series that needs
>  review.  Using patchwork, I can see some recent patch series; or
>  using a hierarchical threaded mail reader, I can find a neglected
>  thread or one that seems to be likely to have an interesting
>  discussion going on.  But without reading in detail, there is no
>  easy way to see whether the series has reached a review, whether
>  someone else intends to review it, and what the author believes its
>  status to be.
> 
> - Relatedly, as a patch author or reviewer, I want to be able to
>  easily tell whether a topic has been sufficiently reviewed.  Today,
>  the signals for this are implicit: I have to judge consensus, or to
>  check the Git repository for whether the patch has been merged, or
>  to check the maintainer's latest "What's cooking in git.git"
>  message.
> 
> - As a potential reviewer or interested user, I want to be able to
>  follow all relevant discussion for a patch series, while also
>  having the ability to stop following it if the discussion goes on
>  too long and starts overwhelming my email inbox.  Today, I can join
>  the discussion and then (1) it is hit-or-miss whether the patch
>  author ccs me on later iterations of the patch and (2) there is no
>  easy way without aggressive email filtering to stop watching it if
>  I am cc-ed.
> 
> - After having diagnosed an issue to be due to a patch, I want to be
>  able to easily find all relevant review discussion.  Today I can
>  use the mailing list archive[4] or patchwork to find review
>  discussion on the latest version of the series that patch was in,
>  but tracing back to previous iterations of that same series can be
>  non-trivial.  Moreover, if I'm interested in a particular puzzling
>  line of code, finding which iteration introduced it can take a long
>  time.

This is a great initiative!

While I do not have anything new to add in terms of pain
points, I just wanted to let you know that this is definitely
something that would have eased the process of bringing in a
new contributor like me.

> Those four are important in my everyday life.  Questions:
> 
> 1. What pain points in the patch flow for git.git are important to
>    you?

As a new contributor (and also someone new to the patch flow) I
would have especially liked the second and fourth point addressed.
When I was preparing my GSoC proposal, I wanted to gather the
status of a previous contributor's work and even though searching
the mailing list helped, it was hard to immediately know what were
the status of the patches, and which changes got introduced in
which version of the patch series.

Also with my first patch series that I sent to the mailing list,
I initially felt unsure about what the status of my patch was
after a few people discussed over it. The 'implicit signals' is
something that was not immediately obvious to me, and only after
reading other interactions in the mailing list did I start getting
a hold of how I should interpret the responses, and what my next
action should be.

> 2. What tricks do you use to get by with those existing pain points?

In order to learn about a previous patch series and what was added,
I used git blame on the relevant part of the codebase, and tried to
search the commit message in the mailing list archive. From there on
it was just opening a ton of tabs in order to see how the patches
developed over time.

The limitation with this trick is it will work only if the patch
actually landed in the codebase. A part of building my proposal
required me to read a patch that did not get merged, and I had to
just aggressively search the mailing list and hope I managed to catch
everything I wanted.

> 3. Do you think patchwork goes in a direction that is likely to help
>    with these?

I have noticed that a patchwork instance for this mailing list
already exists[1] so I decided to try it out. It definitely
addresses the problem of explicitly identifying the status of a
patch. I also liked that I could search for the previous
contributor that I spoke of and sort his contributions by date.
If I knew this existed, I would have saved a lot of time.

But as you also mentioned, it does not yet help me locate an
older version of a particular patch, and let me observe how it
developed over time. So that would definitely be a welcome
addition.

As Bagas mentioned in the thread, it seems to lend itself well
to identify beginner-friendly tasks. I did not personally have
too much difficulty with those thanks to the GitHub issue tracker
and the Git documentation, but if new contributors are anyway
going to refer to patchwork to study previous patches and learn
from it, it might be helpful to keep beginner issues accessible
there as well. Even a generic labelling system to help categorise
issues will do (a downside being that the labels will have to be
maintained and managed too).

Some small nits:
- The searching capability was not super obvious to me. My
  eyes naturally scan for a search box or search icon, it
  took me a few minutes of fiddling to realise that
  'show patches with' is a link that opens all the search
  filters.
- It would be nice (though probably out of scope) to allow
  me to do a code search in the patches.

> 4. What other tools would you like to see that could help?

(...I don't really have an opinion on this)

> Thanks,
> Jonathan
> 
> [1] http://jk.ozlabs.org/projects/patchwork/; you can see an instance
> for Git at https://patchwork.kernel.org/project/git/list/
> [2] https://kernel-recipes.org/en/2016/talks/patches-carved-into-stone-tablets/,
> https://www.kernel.org/pub/software/scm/git/docs/gitworkflows.html#_patch_workflow
> [3] https://colabti.org/irclogger/irclogger_log/git-devel?date=2021-04-12#l40
> [4] https://lore.kernel.org/git/

[1] https://patchwork.kernel.org/project/git/list/