git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Dragan Simic <dsimic@manjaro.org>
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: Fri, 05 Apr 2024 22:40:02 -0700	[thread overview]
Message-ID: <xmqqwmpbm4lp.fsf@gitster.g> (raw)
In-Reply-To: <8a9f4927aab96f2f62e2467e59fb6150d7e931fc.1712367983.git.dsimic@manjaro.org> (Dragan Simic's message of "Sat, 6 Apr 2024 03:48:28 +0200")

Dragan Simic <dsimic@manjaro.org> writes:

> Following the approach of making the produced output more readable, also
> emit additional vertical whitespace after the "Send this email [y/n/...]?"
> prompt.

Hmph.  I'd prefer to see you try not to endlessly extend the scope
of a topic.

By including the above change, the patch no longer is small and
focused enough, which was the reason why we said that the "let's
move the final newline out of the translatable string" can be done
as a "while at it" change.

Besides, because of the switch to separator semantics, that hunk
lost the reason to exist as part of the "use a blank line between
output for each message"---the change no longer is needed to support
the feature.

Even though it is a good change to have, and it deserves to be
justified by its merit alone.

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.

I haven't formed an opinion on that last step, and I do not think I
can spend any time to think about that new part of the feature for
some time (others can review that part and give their opinion on it,
of course, while I'll be working on other topics).  It would mean
you are adding yet another feature to delay the base improvement to
stabilize.  You really do not want to do that.

In any case, this [v4], as a single ball of wax, is not something I
can confidently say "I reviewed this and looks OK".

Thanks.


  reply	other threads:[~2024-04-06  5:40 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 [this message]
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

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=xmqqwmpbm4lp.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=code@khaugsbakk.name \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    /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).