Re: Cc'ing the Git maintainer on GitGitGadget contributions, was Re: [PATCH 0/1] add--interactive: skip index refresh in reset patch mode

4 messages, 3 authors, 2021-01-10 · open the first message on its own page

Re: Cc'ing the Git maintainer on GitGitGadget contributions, was Re: [PATCH 0/1] add--interactive: skip index refresh in reset patch mode

From: Junio C Hamano <hidden>
Date: 2021-01-07 21:26:07

Johannes Schindelin [off-list ref] writes:
I would not call that a fix. As mentioned, I could not come up with a way
to Cc: Junio only when appropriate in a way that wouldn't surprise new
users. So yes, I disabled the auto-Cc:ing, with no appropriate
replacement.
quoted
I.e. now users need to explicitly add "cc: gitster@pobox.com" to the
description, no? So isn't in the same as for us who use the
format-patch/send-email flow in this regard?
Right. And that is... intuitive? If you have to read the manual, the
design is broken.
Let's step back and think when a patch submitter wants to CC
anybody.  People explicitly Cc area experts when touching certain
part of the codebase, so that they can ask for their input.  People
also explicitly Cc the maintainer when they see the reviewers
reached a consensus that it is a good idea to apply the patch.
These CC has value only because they came from conscious decision
by the submitter---they actively asks for help and that nudges the
recipient of CC to help them.

Even without such Cc:, stakeholders (both area experts and the
maintainer) often notice noteworthy patches and discussions, so
between having CC that is thrown around freely without thought and
crowds recipient's inbox and not having such thoughtless CC at all,
the latter is vastly preferred.  The former just diminishes the
value of Cc that are added manually by thoughtful people.  

In short, "now users need to explicitly add" is a GOOD thing.  

The best is to have only thoughtful CC, the second best is not to
have automated CC at all, and the worst is always throw automated CC
to hurt others who try to make CC mean something by making judicious
use of it.

Having said all that.

We may not be able to automate the thinking part to decide when
submitter may want to get help, but an automation can help by giving
the submitter cues and clues when to ask for help and from whom.

Is there a point in the end-user experience flow, starting at the
time when they push their proposed change to their repository, throw
a pull request at GitHub, say "/submit", and then GGG finally sends
out a patch e-mail, where the GGG machinery can inspect the change
and give the user (preferrably before the user says /submit) a hint
that says "you may want to add Cc: to these people in such and such
case, and if you think the situation falls into these cases, tell me
so by saying /submit-with-suggested-cc instead of the usual
/submit"?

And "these people" may include those who touched the affected code
within the past 6 months, and it may also include the maintainer
when the pull request is in its second or later iteration.

Thanks.

Re: Cc'ing the Git maintainer on GitGitGadget contributions, was Re: [PATCH 0/1] add--interactive: skip index refresh in reset patch mode

From: Johannes Schindelin <hidden>
Date: 2021-01-08 14:57:58

Hi Junio,

On Thu, 7 Jan 2021, Junio C Hamano wrote:
We may not be able to automate the thinking part to decide when
submitter may want to get help, but an automation can help by giving
the submitter cues and clues when to ask for help and from whom.
I fear that we're striking the balance on the side of expecting too much
knowledge about project-specific lore from contributors.

My goal with GitGitGadget was to allow competent contributors to focus on
addressing their pet peeve with Git, to produce a stellar patch with an
outstanding description and get that integrated into the next major Git
version, and not have to learn about the culture and mechanics of our
patch review process for what may very well be their single contribution
to Git.

Just look at the wall of text GitGitGadget first-time users _already_ have
to read through, and it does not even catch half of what is relevant for
every single contributor:
https://github.com/gitgitgadget/gitgitgadget/blob/HEAD/res/WELCOME.md

Note that this wall of text does not even begin to answer questions such
as "how do I know when this patch is accepted?".

The suggestion that the contributor should Cc: you explicitly when the
contribution is supposedly ready to advance (according to a set of rules
that is not formalized anywhere that I know of) could potentially be added
to that wall of text, but I doubt that it will make it easier to
contribute even for the most advanced and dedicated software engineers.
Is there a point in the end-user experience flow, starting at the
time when they push their proposed change to their repository, throw
a pull request at GitHub, say "/submit", and then GGG finally sends
out a patch e-mail, where the GGG machinery can inspect the change
and give the user (preferrably before the user says /submit) a hint
that says "you may want to add Cc: to these people in such and such
case, and if you think the situation falls into these cases, tell me
so by saying /submit-with-suggested-cc instead of the usual
/submit"?

And "these people" may include those who touched the affected code
within the past 6 months, and it may also include the maintainer
when the pull request is in its second or later iteration.
We already have a ticket suggesting to add reviewers:
https://github.com/gitgitgadget/gitgitgadget/issues/219

With this suggestion, too, we could go and extend that wall of text even
further and expect contributors to just know what they are supposed to do.
But I don't see how that would make this process more inviting to new
contributors.

BTW I get the sense that many Git mailing list regulars have this idea
that making the review process easier for one-time contributors would
invite too many low-quality contributions. However, I see the exact
opposite: professional software engineers I talk to refuse to jump through
all those hoops, even if they would be quite capable of producing a
stellar patch within no time. At the same time I see a lot of less
experienced developers delighting in the struggle^Wchallenge to set up
`git send-email` correctly _just_ to earn bragging rights to have their
typo fix in Git's commit history. I am exaggerating here, of course, to
make a point. And I think this is a very valid point. We lose
contributions (and potential future reviewers) because we make it harder
than necessary to contribute to Git.

Ciao,
Dscho

Re: Cc'ing the Git maintainer on GitGitGadget contributions, was Re: [PATCH 0/1] add--interactive: skip index refresh in reset patch mode

From: Taylor Blau <hidden>
Date: 2021-01-08 20:09:32

On Fri, Jan 08, 2021 at 03:56:20PM +0100, Johannes Schindelin wrote:
Hi Junio,

On Thu, 7 Jan 2021, Junio C Hamano wrote:
quoted
We may not be able to automate the thinking part to decide when
submitter may want to get help, but an automation can help by giving
the submitter cues and clues when to ask for help and from whom.
I fear that we're striking the balance on the side of expecting too much
knowledge about project-specific lore from contributors.
I think that this could be reasonably addressed. When someone opens a PR
(but before the hit /submit), GGG could say:

    Your change touches these files, and so suggested reviewers include
    X, Y, Z. When you believe your submission is in its last round,
    please also include the maintainer, M.
We already have a ticket suggesting to add reviewers:
https://github.com/gitgitgadget/gitgitgadget/issues/219

With this suggestion, too, we could go and extend that wall of text even
further and expect contributors to just know what they are supposed to do.
But I don't see how that would make this process more inviting to new
contributors.
Yeah, I agree that adding this as a separate step does not make sense,
since it's hard to discover such things (especially by individuals who
merely want to send a single contribution to the project). Having this
happen automatically upon creating a PR would make more sense to me.

Thanks,
Taylor

Re: Cc'ing the Git maintainer on GitGitGadget contributions, was Re: [PATCH 0/1] add--interactive: skip index refresh in reset patch mode

From: Johannes Schindelin <hidden>
Date: 2021-01-10 12:23:32

Hi Taylor,

On Fri, 8 Jan 2021, Taylor Blau wrote:
On Fri, Jan 08, 2021 at 03:56:20PM +0100, Johannes Schindelin wrote:
quoted
On Thu, 7 Jan 2021, Junio C Hamano wrote:
quoted
We may not be able to automate the thinking part to decide when
submitter may want to get help, but an automation can help by giving
the submitter cues and clues when to ask for help and from whom.
I fear that we're striking the balance on the side of expecting too much
knowledge about project-specific lore from contributors.
I think that this could be reasonably addressed. When someone opens a PR
(but before the hit /submit), GGG could say:

    Your change touches these files, and so suggested reviewers include
    X, Y, Z. When you believe your submission is in its last round,
    please also include the maintainer, M.
That is an option.

Is it the best option to reach the goal where competent software engineers
can focus on improving Git's source code?

Maybe it is not possible do automate what I wish for.
quoted
We already have a ticket suggesting to add reviewers:
https://github.com/gitgitgadget/gitgitgadget/issues/219

With this suggestion, too, we could go and extend that wall of text even
further and expect contributors to just know what they are supposed to do.
But I don't see how that would make this process more inviting to new
contributors.
Yeah, I agree that adding this as a separate step does not make sense,
since it's hard to discover such things (especially by individuals who
merely want to send a single contribution to the project). Having this
happen automatically upon creating a PR would make more sense to me.
Right.

I always have this contribution in mind: Improve the readability of `log
--graph` output (https://github.com/gitgitgadget/git/pull/383)

This was an excellent contribution. I doubt that we would have received it
without GitGitGadget, as the contributor could really focus on the code
change instead of the contribution logistics.

We did not exactly make it easy, and I fear that we are losing a lot of
potential contributions because of that.

For example, Git is often ridiculed for being hard to use, and I can
understand. There has been research into ways to improve that, but little
of that research resulted in contributions on the Git mailing list. I
would not at all be surprised if that was because of the review process we
put up.

Usability is only one area where I think we would benefit from attracting
talent. Accessibility is another one. Or UI improvements (consistency,
self-explanatory navigation, etc). Or...

Ciao,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help