git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Charvi Mendiratta <charvi077@gmail.com>
Cc: git <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>,
	Phillip Wood <phillip.wood123@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit
Date: Mon, 22 Feb 2021 09:35:41 -0800	[thread overview]
Message-ID: <xmqqft1o80pe.fsf@gitster.g> (raw)
In-Reply-To: <CAPSFM5dZ=CR21eqE7Y-4AssD9h0ddnUYpy4PSzWVaf8kzsLv_g@mail.gmail.com> (Charvi Mendiratta's message of "Sun, 21 Feb 2021 14:50:05 +0530")

Charvi Mendiratta <charvi077@gmail.com> writes:

>>  So, if it is more work to make the code notice when
>> these options are given in useless combinations and stop with an
>> error message
>
> Sorry I didn't get this and would like to once confirm here that, are
> you pointing to output an error message when the `-m`/`-F` option is
> passed with `git --fixup=amend/reword` ? Because I think we can do
> this also. Otherwise ....

If we were to make -m/-F incompatible with these new features, then
sure, we'd notice the combination, show an error message and abort.

>>than just accepting and doing useless thing, I am OK
>> if we left them as they are.
>
> ....If we allow both `m` and `F` to work with `git commit
> --fixup=amend/reword` with the same working as it is doing now i.e to
> use `m` to write new commit message, upon `--autosquash`, If it is
> okay? then I also agree to update the documentation more precisely and
> include the uses when passed with `m` /`F`(not yet added) option.

What would that more precise documentation would say, though?  

"'-m message' gets appended to the message taken from the original
commit"?  Saying that alone, without explaining why doing such an
appending is useful, would puzzle users and makes them ask "but why
such a useless feature exist?" and that was why I was trying to
figure out what it is useful for with you, which I think we have
failed to do so far.

My preference at this point is to error out the combination that we
cannot figure out how it could be useful at this moment, so that
users who find how it would be useful to come to us and present a
hopefully good case for using -m <msg> with --fixup=amend:<commit>.
I am assuming that allowing the combination at that point is easy,
and the user request will give us a good use case we can use in the
documentation to explain for what purpose a user may want to use -m
<msg> to append a short string at the end.  The end users' use case
we see at that point might even suggest that it would be more useful
to prepend (as opposed to append) the message we get from -m <msg>
to the original log message, and such a change will not be possible
if we just choose to append without thinking through the use case we
intend to support and release "we do not know what good it would do
to append with -m <msg>, but that is what the code happens to do now"
version to the users, as people will depend on the behaviour of any
released versions.

  reply	other threads:[~2021-02-22 17:37 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-17  7:29 [PATCH 0/6][Outreachy] commit: Implementation of "amend!" commit Charvi Mendiratta
2021-02-17  7:37 ` [PATCH 1/6] sequencer: export subject_length() Charvi Mendiratta
2021-02-17  7:37   ` [PATCH 2/6] commit: add amend suboption to --fixup to create amend! commit Charvi Mendiratta
2021-02-17 19:49     ` Junio C Hamano
2021-02-18 10:13       ` Charvi Mendiratta
2021-02-18 19:18         ` Junio C Hamano
2021-02-18 20:37           ` Junio C Hamano
2021-02-19  6:10             ` Charvi Mendiratta
2021-02-19  6:09           ` Charvi Mendiratta
2021-02-20  3:15             ` Junio C Hamano
2021-02-21  6:35               ` Charvi Mendiratta
2021-02-21  7:05                 ` Junio C Hamano
2021-02-21  9:20                   ` Charvi Mendiratta
2021-02-22 17:35                     ` Junio C Hamano [this message]
2021-02-23  6:05                       ` Charvi Mendiratta
2021-02-17  7:37   ` [PATCH 3/6] commit: add a reword suboption to --fixup Charvi Mendiratta
2021-02-17 19:56     ` Junio C Hamano
2021-02-18 10:14       ` Charvi Mendiratta
2021-02-17  7:37   ` [PATCH 4/6] t7500: add tests for --fixup[amend|reword] options Charvi Mendiratta
2021-02-17 19:59     ` Junio C Hamano
2021-02-18 10:15       ` Charvi Mendiratta
2021-02-18 19:26         ` Junio C Hamano
2021-02-19  6:10           ` Charvi Mendiratta
2021-02-17  7:37   ` [PATCH 5/6] t3437: use --fixup with options to create amend! commit Charvi Mendiratta
2021-02-17  7:37   ` [PATCH 6/6] doc/git-commit: add documentation for fixup[amend|reword] options Charvi Mendiratta
2021-02-18 19:23     ` Junio C Hamano
2021-02-19  6:09       ` Charvi Mendiratta
2021-02-23 19:55 ` [PATCH 0/6][Outreachy] commit: Implementation of "amend!" commit Junio C Hamano
2021-02-24  5:54   ` Charvi Mendiratta
2021-02-25 10:08 ` [PATCH v2 " Charvi Mendiratta
2021-02-25 10:08 ` [PATCH v2 1/6] sequencer: export subject_length() Charvi Mendiratta
2021-02-25 10:08 ` [PATCH v2 2/6] commit: add amend suboption to --fixup to create amend! commit Charvi Mendiratta
2021-02-25 21:00   ` Junio C Hamano
2021-02-26 10:38     ` Charvi Mendiratta
2021-02-26 19:32       ` Junio C Hamano
2021-02-27  4:56         ` Charvi Mendiratta
2021-02-25 10:08 ` [PATCH v2 3/6] commit: add a reword suboption to --fixup Charvi Mendiratta
2021-02-25 20:32   ` Junio C Hamano
2021-02-26 10:35     ` Charvi Mendiratta
2021-02-25 10:09 ` [PATCH v2 4/6] t7500: add tests for --fixup=[amend|reword] options Charvi Mendiratta
2021-02-25 10:09 ` [PATCH v2 5/6] t3437: use --fixup with options to create amend! commit Charvi Mendiratta
2021-02-25 10:09 ` [PATCH v2 6/6] doc/git-commit: add documentation for fixup=[amend|reword] options Charvi Mendiratta
2021-02-25 20:48   ` Junio C Hamano
2021-02-26 10:36     ` Charvi Mendiratta

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=xmqqft1o80pe.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=charvi077@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.com \
    --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.
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).