git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Dragan Simic <dsimic@manjaro.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, code@khaugsbakk.name
Subject: Re: [PATCH v4] send-email: make it easy to discern the messages for each patch
Date: Mon, 08 Apr 2024 18:15:48 +0200	[thread overview]
Message-ID: <d334e0cc3a681d7ec5b06edac7d21232@manjaro.org> (raw)
In-Reply-To: <xmqqttkbetd3.fsf@gitster.g>

Hello Junio,

On 2024-04-08 18:01, Junio C Hamano wrote:
> Dragan Simic <dsimic@manjaro.org> writes:
> 
>> On 2024-04-06 19:05, Junio C Hamano wrote:
>>> Junio C Hamano <gitster@pobox.com> writes:
>>> 
>>>> The whole thing deserves to be a three-patch series, the first one
>>>> being a preliminarly "let's move the final newline out of the
>>>> translatable string" step, followed by "let's have a gap between
>>>> output for each patch sent out".  Perhaps another "even during
>>>> sending a single patch, we may want extra blank lines when use of
>>>> editor and other user interation is involved" patch on top.
>>> Or the latter two could be done in a single patch.  I'll have to
>>> re-review the thing (if I were the only reviewer of the topic) so
>>> doing so would delay the completion of the topic, though.
>> 
>> Huh, I've already separated this patch into three patches, and IMHO
>> they look nice and make everything less error prone.  Would you agree
>> with the three-patch approach, please?
> 
> My "or the latter two could be done in a single patch" was
> "alternatively you can", so either way is fine as long as the result
> is well structured.

Great, thanks.  I really appreciate your highly detailed approach
to reviewing the patches.

> I know how to explain "insert a gap between
> patches" well.  I do not know which one is easier to explain,
> between
> 
>  (1) now we have "insert a gap between patches" with patch [2/3],
>      but when editor invocation and confirmation prompts are
>      involved, there are these three cases where we want to tweak
>      the logic to show gaps.  Here in patch [3/3] I explain how each
>      of these three affect the logic from the previous step.

In my opinion, the insertion of vertical whitespace can also be
seen rather independently when it comes to separating the patches,
and separating the prompts from the patches.

>  (2) We want to insert a gap before showing the second and
>      subsequent patches, unless in such and such cases.  We also
>      want to insert a gap when we do this and that.  We do all of
>      these in this patch [2/2].
> 
> So doing it in three-patches results in a series easier to
> understand by readers, by all means, please do.

As I mentioned above, the rather independent nature of the insertion
of the two "classes" of vertical whitespace makes the three-patch
approach look more suitable to me.  I think it's also a better option
if some regression is discovered later, because the patches can be
applied and tested separately.

I already went ahead and submitted the three-patch series, [1] please
have a look.  I hope you'll like it.

[1] 
https://lore.kernel.org/git/cover.1712486910.git.dsimic@manjaro.org/T/#u


      reply	other threads:[~2024-04-08 16:16 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-06  1:48 [PATCH v4] send-email: make it easy to discern the messages for each patch Dragan Simic
2024-04-06  5:40 ` Junio C Hamano
2024-04-06  8:56   ` Dragan Simic
2024-04-06 10:18     ` Dragan Simic
2024-04-06 17:05   ` Junio C Hamano
2024-04-06 17:08     ` Dragan Simic
2024-04-08 16:01       ` Junio C Hamano
2024-04-08 16:15         ` Dragan Simic [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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=d334e0cc3a681d7ec5b06edac7d21232@manjaro.org \
    --to=dsimic@manjaro.org \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).