Hi Junio
On 22/05/2022 05:32, Junio C Hamano wrote:
A typical "git revert" commit uses the full title of the original
commit in its title, and starts its body of the message with:
This reverts commit 8fa7f667cf61386257c00d6e954855cc3215ae91.
This does not encourage the best practice of describing not just
"what" (i.e. "Revert X" on the title says what we did) but "why"
(i.e. and it does not say why X was undesirable).
We can instead phrase this first line of the body to be more like
This reverts commit 8fa7f667 (do this and that, 2022-04-25)
so that the title does not have to be
Revert "do this and that"
We can instead use the title to describe "why" we are reverting the
original commit.
Introduce the "--reference" option to "git revert", and also the
revert.reference configuration variable, which defaults to false, to
tweak the title and the first line of the draft commit message for
when creating a "revert" commit.
I think this is a good idea which will hopefully improve project histories.
diff --git a/builtin/revert.c b/builtin/revert.c
index 51776abea6..ada51e46b9 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -116,6 +116,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
N_("option for merge strategy"), option_parse_x),
{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
+ OPT_BOOL(0, "reference", &opts->commit_use_reference,
+ N_("use the 'reference' format to refer to commits")),
This option is being added to base_options which applies to cherry-pick
as well as revert. There is a "if" statement just below this hunk which
adds cherry-pick specific options, I think we want to add this new
option in an else block added to that "if" statement.
static int do_pick_commit(struct repository *r,
struct todo_item *item,
struct replay_opts *opts,
@@ -2167,14 +2184,19 @@ static int do_pick_commit(struct repository *r,
base_label = msg.label;
next = parent;
next_label = msg.parent_label;
- strbuf_addstr(&msgbuf, "Revert \"");
- strbuf_addstr(&msgbuf, msg.subject);
- strbuf_addstr(&msgbuf, "\"\n\nThis reverts commit ");
- strbuf_addstr(&msgbuf, oid_to_hex(&commit->object.oid));
+ if (!opts->commit_use_reference) {
+ strbuf_addstr(&msgbuf, "Revert \"");
+ strbuf_addstr(&msgbuf, msg.subject);
+ strbuf_addstr(&msgbuf, "\"");
+ } else {
+ strbuf_addstr(&msgbuf, "DESCRIBE WHY WE ARE REVERTING HERE");
In format-patch we add three asterisks to the beginning and end of the
dummy subject and body lines to encourage the user to edit them - is
that worth doing here as well?
Best Wishes
Phillip
