From: Jeff King <peff@peff.net>
To: phillip.wood@dunelm.org.uk
Cc: Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org,
Jonas Kittner <jonas.kittner@ruhr-uni-bochum.de>,
Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] rebase -i: fix rewording with --committer-date-is-author-date
Date: Wed, 3 Nov 2021 07:42:36 -0400 [thread overview]
Message-ID: <YYJ1rA8F22SSBeNS@coredump.intra.peff.net> (raw)
In-Reply-To: <101071b2-0b7d-5ee8-ca81-171e08a1ffdf@gmail.com>
On Wed, Nov 03, 2021 at 11:23:28AM +0000, Phillip Wood wrote:
> > That description makes sense, and the patch matches. Not being that
> > familiar with this area, my biggest question would be: are there are
> > other cases that would need the same treatment? And is there a way we
> > can make it easier to avoid forgetting such a case in the future?
>
> I don't think there are any other cases (but then I thought that when I
> wrote the buggy patch...). The only time we change the authorship is if the
> user passes --committer-date-is-author-date or --reset-author-date. I agree
> it would be good to have a way to avoid this problem in the future but I
> haven't come up with an easy way to do that. One possibility would be to go
> back to always reading the author script. That would mean revisiting the
> changes to do_merge() in baf8ec8d3a so that it always writes the author
> script and .git/MERGE_MSG but removes them when fast-forwarding (the problem
> that baf8ec8d3a tried to solve was a left over .git/MERGE_MSG when
> do_merge() fast-forwarded) I don't want to do that in the rc window though.
I suspected the answers to my questions were "I hope so" and "not
really", which I think matches what you wrote. :)
If there isn't an easy way to make it more future-proof, then I'm
content that you've given it some thought and didn't find any other
cases. We can proceed from here with this fix, and be on the lookout for
any other cases that people report (on the plus side, the BUG() made it
quite obvious that there was a problem, rather than a subtle behavior
change).
> Thanks for your comments, are you happy for this to go in as is or should I
> look at simplifying the conditional?
I'm happy enough with it. I don't know what the plan is for the -rc
period, though. AFAICT the bug is in v2.33.1, so it's not technically a
v2.34-rc problem. It could wait for the next maint release.
-Peff
next prev parent reply other threads:[~2021-11-03 11:43 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-02 20:10 [PATCH] rebase -i: fix rewording with --committer-date-is-author-date Phillip Wood via GitGitGadget
2021-11-02 21:05 ` Eric Sunshine
2021-11-02 21:29 ` Phillip Wood
2021-11-02 21:30 ` [PATCH v2] " Phillip Wood via GitGitGadget
2021-11-02 22:32 ` [PATCH] " Jeff King
2021-11-03 11:23 ` Phillip Wood
2021-11-03 11:42 ` Jeff King [this message]
2021-11-03 17:44 ` Junio C Hamano
2021-11-04 2:03 ` Jeff King
2021-11-04 6:27 ` 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=YYJ1rA8F22SSBeNS@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=jonas.kittner@ruhr-uni-bochum.de \
--cc=phillip.wood@dunelm.org.uk \
/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.