git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: Derrick Stolee <stolee@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] ls-refs: reuse buffer when sending refs
Date: Wed, 25 Aug 2021 10:23:53 -0700	[thread overview]
Message-ID: <xmqqzgt5igk6.fsf@gitster.g> (raw)
In-Reply-To: <YSZlK2y74wmYQcSd@ncase> (Patrick Steinhardt's message of "Wed, 25 Aug 2021 17:43:39 +0200")

Patrick Steinhardt <ps@pks.im> writes:

> I don't know. I just happen to revisit this topic every few days, and
> every time I stumble upon some more performance improvements. It would
> feel wrong to shift the goalposts of the other series every time I find
> something new, so I instead opt for separate patch series.
>
> If this proves to be annoying for reviewers, then feel free to shout at
> me and I'll change my approach.

The easiest would be to keep them independent and to justify them
independently.  There is no shifting the goalposts involved if you
did so.  Of course, you wouldn't be able to include the improvements
the follow-on topics make as part of the "advertising material" to
sell the benefit of the base series, though.  It can only work when
the follow-on topic is truly independent and is a good idea by
itself.  Another is to keep these follow-on topics unpublished
before the base topic graduates, and pretend that you came up with
them much later than you originally did.  Nobody would notice and
mind, as the base topic would be cast in stone by that time.

At the receiving end, what is most irritating is a series of topics
that pretends to be about different things but depend on each other
to function well [*].  I would imagine that it would be a lot more
trivial and pleasant to handle if any of the patches involved did
not come before these follow-on topics are all already thought of by
the author and instead came in a well-structured single topic, but
we do not live in a perfect world ;-).

In the case of this particular patch, I think the logic behind the
change makes sense by itself, so if I were doing it, I'd probably
choose to sell it as an independent change unrelated to the other
topic.

Thanks.


[Footnote]

* Any time the base topic gets rerolled, I'd be the one who ends up
  having to remember which other topics that did not rerolled depend
  on it in what order and rebase them correctly, and then I have to
  replace the follow-on topics that have been rerolled.  Even a
  single missed subtopic will cause the day's integration work
  redone, and all that robs time and concentration that the topics
  by other contributors needs from me, which would make me grumpy
  X-<.

      reply	other threads:[~2021-08-25 17:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-25 13:49 [PATCH] ls-refs: reuse buffer when sending refs Patrick Steinhardt
2021-08-25 14:10 ` Patrick Steinhardt
2021-08-25 14:50 ` Derrick Stolee
2021-08-25 15:43   ` Patrick Steinhardt
2021-08-25 17:23     ` Junio C Hamano [this message]

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=xmqqzgt5igk6.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=stolee@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).