All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kim Altintop <kim@eagain.st>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: git@vger.kernel.org, gitster@pobox.com, jonathantanmy@google.com,
	bwilliams.eng@gmail.com, jrnieder@gmail.com,
	sunshine@sunshineco.com
Subject: Re: [PATCH v6 0/3] upload-pack: treat want-ref relative to namespace
Date: Sun, 15 Aug 2021 19:35:04 +0000	[thread overview]
Message-ID: <20210815213453.GB10013@schmidt.localdomain> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.2108142344570.59@tvgsbejvaqbjf.bet>

Hi Johannes,

thanks for your response. This is my very first patch to git.git, and a lot of
it is learning about the conventions and expectations towards contributors, as
well as getting familiar with the tooling while trying to avoid silly mistakes.
So please bear with me.

On Sat, 14 Aug 2021 Johannes Schindelin wrote:

> > CC'ing Johannes Schindelin as suggested by git-contacts.
>
> `git-contacts` wouldn't know that there are better experts on the
> namespace matter.

I can see now that this could come across weird. I should've written:

"CC'ing Johannes Schindelin, who started to turn up in `git-contacts` output,
although I couldn't quite infer why. I haven't received any feedback about the
documentation change yet, and didn't have much success trying to find reviewers
by inspecting the history (parts of the file where moved). I am assuming that
`git-contacts` is better than me at this, and Johannes' name shows up because of
touching the documentation. Johannes: feel free to ignore if this assumption is
wrong."

With this said, if you have any suggestions about finding reviewers for specific
parts of a patch, or who are the experts on a more cross-cutting topic, I would
appreciate if you'd share them!

> My only comment is that I would find the diff to `upload-pack.c` much
> easier to parse if the `arg` variable hadn't been renamed.

Can you explain why? Just because the diff would be smaller? I can see that in a
larger patch it might have been preferable to put the rename into a separate
commit, but in a hunk-sized change it seemed fine. It is also that this
particular naming ("refname_nons") is used in other places in upload-pack.c, so
it seemed obvious that, if I introduce namespace handling where it was
previously missing, the terminology (if you will) should be the same.

From you comment, it seems like the proposer of a patch should assume that the
reviewers only look at the diff as sent in the email, and not any context.
Junio's response suggests something else, but I guess it's fair that if someone
feels like they got CC'ed by mistake, they're not going to spend too much time.

So my question from above stands: are there better ways to find the right people
to CC, especially for newbies?

Thanks,
Kim


  parent reply	other threads:[~2021-08-15 19:35 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 13:59 [PATCH] upload-pack.c: treat want-ref relative to namespace Kim Altintop
2021-07-30 14:04 ` Kim Altintop
2021-07-30 18:57 ` Junio C Hamano
2021-07-30 21:08   ` Kim Altintop
2021-07-31 20:36 ` [PATCH v2] " Kim Altintop
2021-08-02 21:06   ` Jonathan Tan
2021-08-04 20:36     ` Kim Altintop
2021-08-04 20:42   ` [PATCH v3] " Kim Altintop
2021-08-04 21:00     ` [PATCH v4] " Kim Altintop
2021-08-09 17:56       ` [PATCH 0/3] upload-pack: " Kim Altintop
2021-08-09 17:56         ` [PATCH 1/3] t5730: introduce fetch command helper Kim Altintop
2021-08-09 19:16           ` Junio C Hamano
2021-08-09 21:18             ` Kim Altintop
2021-08-09 19:40           ` Jonathan Nieder
2021-08-09 21:43             ` Junio C Hamano
2021-08-09 21:56             ` Kim Altintop
2021-08-09 22:03               ` Junio C Hamano
2021-08-09 23:01                 ` Jonathan Nieder
2021-08-10  9:44                   ` Kim Altintop
2021-08-09 17:57         ` [PATCH 2/3] upload-pack.c: treat want-ref relative to namespace Kim Altintop
2021-08-09 17:57         ` [PATCH 3/3] docs: clarify the interaction of transfer.hideRefs and namespaces Kim Altintop
2021-08-10  9:49           ` Kim Altintop
2021-08-13  6:23         ` [PATCH v6 0/3] upload-pack: treat want-ref relative to namespace Kim Altintop
2021-08-14 21:46           ` Johannes Schindelin
2021-08-15 17:59             ` Junio C Hamano
2021-08-15 19:35             ` Kim Altintop [this message]
2021-08-16 12:39               ` Johannes Schindelin
2021-08-13  6:23         ` [PATCH v6 1/3] t5730: introduce fetch command helper Kim Altintop
2021-08-13  6:23         ` [PATCH v6 2/3] upload-pack.c: treat want-ref relative to namespace Kim Altintop
2021-08-13  6:23         ` [PATCH v6 3/3] docs: clarify the interaction of transfer.hideRefs and namespaces Kim Altintop
2021-08-04 21:15     ` [PATCH v3] upload-pack.c: treat want-ref relative to namespace Junio C Hamano
2021-08-04 22:04       ` Kim Altintop
2021-08-04 22:17         ` Eric Sunshine
2021-08-04 22:17         ` Junio C Hamano
2021-08-04 22:23         ` Junio C Hamano

Reply instructions:

You may reply publicly 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

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

  git send-email \
    --in-reply-to=20210815213453.GB10013@schmidt.localdomain \
    --to=kim@eagain.st \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=bwilliams.eng@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=sunshine@sunshineco.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.