From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Kristoffer Haugsbakk <code@khaugsbakk.name>
Subject: Re: [PATCH v2] sequencer: beautify subject of reverts of reverts
Date: Fri, 28 Apr 2023 21:11:57 +0200 [thread overview]
Message-ID: <ZEwafQmat347la3/@ugly> (raw)
In-Reply-To: <xmqqcz3netxr.fsf@gitster.g>
On Fri, Apr 28, 2023 at 11:35:28AM -0700, Junio C Hamano wrote:
>Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:
>> +The command generates the subject 'Revert "<title>"' for the resulting
>> +commit, assuming the original commit's subject is '<title>'. Reverting
>> +such a reversion commit in turn yields the subject 'Reapply "<title>"'.
>
>Clearly written.
>
>> +These can of course be modified in the editor when the reason for
>> +reverting is described.
>
>Not just the title but the entire message can be edited and that is
>by design. Having to modify what this new mechanism does when
>existing users do not like the new behaviour will annoy them, and
>this sentence will not be a good enough excuse to ask them
>forgiveness for breaking their established practice, either.
>
>So, I am not sure if there is a point to have this sentence here.
>
well, it's the one sentence i copied verbatim from your proposal. :-D
but i don't get the argument anyway. i think the docu is pretty
pointless except to emphasize that the generated subject is a default
that should be edited when circumstances recommend it. in fact, i
wouldn't mind writing just that, with a notice that the default attempts
to be somewhat natural for repeated reverts.
>> strbuf_addstr(&msgbuf,
>> "# *** SAY WHY WE ARE REVERTING ON THE TITLE LINE ***");
>> + } else if (skip_prefix(msg.subject, "Revert \"", &orig_subject)) {
>> + if (skip_prefix(orig_subject, "Revert \"", &orig_subject)) {
>> + /*
>> + * This prevents the generation of somewhat unintuitive (even if
>> + * not incorrect) 'Reapply "Revert "' titles from legacy double
>> + * reverts. Fixing up deeper recursions is left to the user.
>> + */
>
>Good comment but in an overwide paragraph.
>
there are several lines in the lower 90-ies in that file, one of them
seen in the patch context. would 88 be fine?
(too narrow flowed text looks silly, imo.)
>Doesn't t3501 seem a better home for them?
>
looking closer at it, i guess it kind of does. the file's contents have
clearly grown to fulfill the filename's broad promise, but nobody
bothered to adjust the test description and make the setup title more
specific. any takers?
-- ossi
next prev parent reply other threads:[~2023-04-28 19:12 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 8:35 [PATCH v2] sequencer: beautify subject of reverts of reverts Oswald Buddenhagen
2023-04-28 18:35 ` Junio C Hamano
2023-04-28 19:11 ` Oswald Buddenhagen [this message]
2023-05-01 16:44 ` Junio C Hamano
2023-05-01 19:10 ` Oswald Buddenhagen
2023-05-01 19:12 ` Junio C Hamano
2023-05-05 17:25 ` Junio C Hamano
2023-05-17 9:05 ` Phillip Wood
2023-05-17 10:00 ` Oswald Buddenhagen
2023-05-17 11:20 ` Phillip Wood
2023-05-17 17:02 ` Oswald Buddenhagen
2023-05-18 9:58 ` Phillip Wood
2023-05-18 16:28 ` Junio C Hamano
2023-07-28 5:26 ` Linus Arver
2023-07-28 9:45 ` Oswald Buddenhagen
2023-07-28 15:10 ` Junio C Hamano
2023-07-28 15:37 ` Oswald Buddenhagen
2023-07-28 16:31 ` Junio C Hamano
2023-07-28 16:47 ` Oswald Buddenhagen
2023-07-28 17:36 ` Linus Arver
2023-08-09 17:15 ` [PATCH v3 1/2] " Oswald Buddenhagen
2023-08-09 17:15 ` [PATCH v3 2/2] doc: revert: add discussion Oswald Buddenhagen
2023-08-10 21:50 ` Linus Arver
2023-08-10 22:00 ` Linus Arver
2023-08-11 15:10 ` Phillip Wood
2023-08-12 6:25 ` Oswald Buddenhagen
2023-08-13 22:09 ` Junio C Hamano
2023-08-14 14:13 ` Oswald Buddenhagen
2023-08-11 12:49 ` Oswald Buddenhagen
2023-08-11 23:00 ` Linus Arver
2023-08-12 7:19 ` Oswald Buddenhagen
2023-09-07 21:29 ` Linus Arver
2023-08-11 15:08 ` Phillip Wood
2023-08-11 17:10 ` Junio C Hamano
2023-08-11 17:05 ` Junio C Hamano
2023-08-11 17:44 ` Re* " Junio C Hamano
2023-08-11 17:53 ` Junio C Hamano
2023-08-11 17:56 ` rsbecker
2023-08-11 18:16 ` Eric Sunshine
2023-08-11 18:16 ` Oswald Buddenhagen
2023-08-11 19:43 ` Junio C Hamano
2023-08-11 15:05 ` [PATCH v3 1/2] sequencer: beautify subject of reverts of reverts Phillip Wood
2023-08-11 16:59 ` Junio C Hamano
2023-08-11 17:13 ` Oswald Buddenhagen
2023-08-21 17:07 ` [PATCH v4 " Oswald Buddenhagen
2023-08-21 17:07 ` [PATCH v4 2/2] git-revert.txt: add discussion Oswald Buddenhagen
2023-08-21 18:32 ` [PATCH v4 1/2] sequencer: beautify subject of reverts of reverts Junio C Hamano
2023-08-23 20:08 ` Taylor Blau
2023-08-23 21:38 ` Junio C Hamano
2023-08-24 6:14 ` Oswald Buddenhagen
2023-09-02 7:20 ` [PATCH v5] " Oswald Buddenhagen
2023-09-02 22:24 ` Junio C Hamano
2023-09-11 20:12 ` Kristoffer Haugsbakk
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=ZEwafQmat347la3/@ugly \
--to=oswald.buddenhagen@gmx.de \
--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.
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).