All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aleen 徐沛文" <pwxu@coremail.cn>
To: "Junio C Hamano" <gitster@pobox.com>
Cc: "Aleen via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	Aleen <aleen42@vip.qq.com>
Subject: Re: [PATCH v6 3/3] am: throw an error when passing --empty option without value
Date: Fri, 19 Nov 2021 10:11:06 +0800 (CST)	[thread overview]
Message-ID: <526ec6c0.87.17d35f63231.Coremail.pwxu@coremail.cn> (raw)
In-Reply-To: <xmqqr1bdrlpq.fsf@gitster.g>

> Here is a place for you to explain why this change might be a good
> idea to defend it.  If we applied 1/3 and 2/3 but not this one, what
> happens?  "am --empty" (not "am --empty=<choice>") is accepted and
> behaves just like "am --empty=die"?  Is that a bad thing?
> 
> If it is unconditonally a bad thing, then this patch does not stand
> on its own.  It should be squashed into the previous step; otherwise
> it would be "oops, we didn't think it through and failed to detect
> an argument-less --error as an error in the previous step, and here
> is a fix for that mistake".  We try not to deliberately commit a
> mistake and then fix it to gain commit counts in this project (or
> putting it differently, we try to pretend we are better than we
> really are and did not make a mistake in the first place).
> 
> Or are you giving list participants and reviewers a choice between
> "'--empty' is a synonym to '--empty=die'" and "'--empty' alone is an
> error" because you cannot decide yourself?  If that is the case, you
> really really need to make it clear in this place between "From:"
> and "Signed-off-by:".  Why would we want to use this patch?  Why
> would we be better without this patch?
> 
> I haven't thought things through, but my gut feeling is that the
> semantics of the --empty=<choice> option is better _with_ this,
> in other words, it makes more sense to forbid "--empty" alone.  So
> if I were doing this series, I would probably have squashed this
> into the previous one, making it a two-patch series.
> 
> Thanks.

Hamano, I could not make this decision like what you said between "'--empty' is a synonym to '--empty=die'" and "'--empty' alone is an error". In my opinion, it is a little wired that '--empty' should not act as "died" semantically. Reversely, it may be better to support a synonym between '--quiet' and '--empty=skip'.

I will squash the commit into the previous one later and hope all participants and reviewers make a decision.

  reply	other threads:[~2021-11-19  2:11 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-12  4:58 [PATCH 0/2] am: support --always option to am empty commits Aleen via GitGitGadget
2021-11-12  4:58 ` [PATCH 1/2] doc: git-format-patch: specify the option --always Aleen via GitGitGadget
2021-11-12  4:58 ` [PATCH 2/2] am: support --always option to am empty commits Aleen via GitGitGadget
2021-11-12  6:17 ` [PATCH 0/2] " René Scharfe
2021-11-12  6:42   ` Aleen
2021-11-12  6:47   ` Junio C Hamano
2021-11-12  7:10     ` Aleen 徐沛文
2021-11-12 15:28     ` René Scharfe
2021-11-12 16:08       ` Junio C Hamano
2021-11-12  6:53 ` [PATCH v2 0/4] am: support --allow-empty " Aleen via GitGitGadget
2021-11-12  6:53   ` [PATCH v2 1/4] doc: git-format-patch: specify the option --always Aleen via GitGitGadget
2021-11-12 22:17     ` Junio C Hamano
2021-11-12  6:53   ` [PATCH v2 2/4] am: support --always option to am empty commits Aleen via GitGitGadget
2021-11-12 22:23     ` Junio C Hamano
2021-11-12  6:53   ` [PATCH v2 3/4] test: am: add the case when not passing the --always option Aleen via GitGitGadget
2021-11-12  6:54   ` [PATCH v2 4/4] chore: am: rename the --always option to --allow-empty Aleen via GitGitGadget
2021-11-15 10:39   ` [PATCH v3 0/2] am: support --empty-commit=(die|skip|asis) option to am empty commits Aleen via GitGitGadget
2021-11-15 10:39     ` [PATCH v3 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-15 10:39     ` [PATCH v3 2/2] am: support --empty-commit option to handle empty patches Aleen via GitGitGadget
2021-11-15 11:13       ` Aleen 徐沛文
2021-11-16  5:18     ` [PATCH v4 0/2] am: support --empty-commit=(die|skip|asis) option to am empty commits Aleen via GitGitGadget
2021-11-16  5:18       ` [PATCH v4 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-16  5:18       ` [PATCH v4 2/2] am: support --empty-commit option to handle empty patches Aleen via GitGitGadget
2021-11-16 10:07         ` Phillip Wood
2021-11-16 10:31           ` Aleen 徐沛文
2021-11-17  8:39           ` Junio C Hamano
2021-11-17  9:33       ` [PATCH v5 0/2] am: support --empty-commit=(die|skip|asis) option to am empty commits Aleen via GitGitGadget
2021-11-17  9:33         ` [PATCH v5 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-17  9:33         ` [PATCH v5 2/2] am: support --empty option to handle empty patches Aleen via GitGitGadget
2021-11-18 10:50         ` [PATCH v6 0/3] am: support --empty=(die|drop|keep) " Aleen via GitGitGadget
2021-11-18 10:50           ` [PATCH v6 1/3] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-18 10:50           ` [PATCH v6 2/3] am: support --empty option to handle empty patches Aleen via GitGitGadget
2021-11-19  0:56             ` Junio C Hamano
2021-11-19 10:33             ` Bagas Sanjaya
2021-11-19 12:10               ` Ævar Arnfjörð Bjarmason
2021-11-19 12:20                 ` Eric Sunshine
2021-11-19 16:49                   ` Junio C Hamano
2021-11-19 16:46               ` Junio C Hamano
2021-11-18 10:50           ` [PATCH v6 3/3] am: throw an error when passing --empty option without value Aleen via GitGitGadget
2021-11-19  1:13             ` Junio C Hamano
2021-11-19  2:11               ` Aleen 徐沛文 [this message]
2021-11-18 23:47           ` [PATCH v6 0/3] am: support --empty=(die|drop|keep) option to handle empty patches Junio C Hamano
2021-11-19  1:45             ` Aleen 徐沛文
2021-11-19  5:46               ` Junio C Hamano
2021-11-19  7:23                 ` Aleen 徐沛文
2021-11-19  7:25                   ` =?gb18030?B?QWxlZW4=?=
2021-11-19 16:54                   ` Junio C Hamano
2021-11-19 17:14                     ` Aleen 徐沛文
2021-11-19 19:25                       ` Junio C Hamano
2021-11-22 11:57                     ` Johannes Schindelin
2021-11-19  4:16             ` Aleen 徐沛文
2021-11-19  5:04           ` [PATCH v7 0/2] " Aleen via GitGitGadget
2021-11-19  5:04             ` [PATCH v7 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-19  5:04             ` [PATCH v7 2/2] am: support --empty=<option> to handle empty patches Aleen via GitGitGadget
2021-11-19 22:50               ` Junio C Hamano
2021-11-19 23:07               ` Junio C Hamano
2021-11-22  6:46             ` [PATCH v8 0/2] am: support --empty=(die|drop|keep) option " Aleen via GitGitGadget
2021-11-22  6:46               ` [PATCH v8 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-22  6:46               ` [PATCH v8 2/2] am: support --empty=<option> to handle empty patches Aleen via GitGitGadget
2021-11-22  7:06                 ` Junio C Hamano
2021-11-22  7:19                   ` Aleen 徐沛文
2021-11-22  7:02               ` [PATCH v9 0/2] am: support --empty=(die|drop|keep) option " Aleen via GitGitGadget
2021-11-22  7:02                 ` [PATCH v9 1/2] doc: git-format-patch: describe the option --always Aleen 徐沛文 via GitGitGadget
2021-11-22  7:02                 ` [PATCH v9 2/2] am: support --empty=<option> to handle empty patches Aleen 徐沛文 via GitGitGadget
2021-11-22  7:04                   ` Aleen 徐沛文
2021-11-22  7:51                 ` [PATCH v10 0/2] am: support --empty=(die|drop|keep) option " Aleen via GitGitGadget
2021-11-22  7:51                   ` [PATCH v10 1/2] doc: git-format-patch: describe the option --always Aleen via GitGitGadget
2021-11-22 12:00                     ` Johannes Schindelin
2021-11-23  1:25                       ` Aleen 徐沛文
2021-11-23 12:30                         ` Johannes Schindelin
2021-11-22  7:51                   ` [PATCH v10 2/2] am: support --empty=<option> to handle empty patches Aleen via GitGitGadget
2021-11-23 15:26                   ` [PATCH v11 0/2] am: support --empty=(die|drop|keep) option " Aleen via GitGitGadget
2021-11-23 15:26                     ` [PATCH v11 1/2] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-11-23 16:12                       ` Johannes Schindelin
2021-11-23 22:02                         ` Junio C Hamano
2021-11-23 15:26                     ` [PATCH v11 2/2] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-11-26 20:14                       ` Elijah Newren
2021-11-29  9:19                         ` Aleen 徐沛文
2021-11-29 10:00                           ` Aleen 徐沛文
2021-11-29 17:10                             ` Elijah Newren
2021-11-30  5:45                               ` [PATCH v12 3/3] am: support --allow-empty to record specific " Aleen 徐沛文
2021-11-29 18:17                         ` [PATCH v11 2/2] am: support --empty=<option> to handle " Junio C Hamano
2021-11-29 18:57                           ` Elijah Newren
2021-11-30  5:37                     ` [PATCH v12 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option " Aleen via GitGitGadget
2021-11-30  5:37                       ` [PATCH v12 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-11-30  5:37                       ` [PATCH v12 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-11-30  5:37                       ` [PATCH v12 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-11-30  7:21                         ` Junio C Hamano
2021-11-30  9:55                       ` [PATCH v13 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-11-30  9:55                         ` [PATCH v13 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-11-30  9:55                         ` [PATCH v13 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-11-30  9:55                         ` [PATCH v13 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-01  3:37                         ` [PATCH v14 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-12-01  3:37                           ` [PATCH v14 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-01  3:37                           ` [PATCH v14 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-03 22:30                             ` Johannes Schindelin
2021-12-01  3:37                           ` [PATCH v14 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-02  0:58                             ` Junio C Hamano
2021-12-06  1:35                               ` Aleen 徐沛文
2021-12-06  9:41                           ` [PATCH v15 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-12-06  9:41                             ` [PATCH v15 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-06  9:41                             ` [PATCH v15 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-06  9:41                             ` [PATCH v15 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-07  5:01                             ` [PATCH v16 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-12-07  5:01                               ` [PATCH v16 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-07  5:01                               ` [PATCH v16 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-07  5:01                               ` [PATCH v16 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-07  8:31                               ` [PATCH v17 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-12-07  8:31                                 ` [PATCH v17 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-07  8:31                                 ` [PATCH v17 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-07 18:12                                   ` Junio C Hamano
2021-12-07  8:31                                 ` [PATCH v17 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-07 18:23                                   ` Junio C Hamano
2021-12-07 18:24                                 ` [PATCH v17 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Junio C Hamano
2021-12-08  5:05                                 ` [PATCH v18 " Aleen via GitGitGadget
2021-12-08  5:05                                   ` [PATCH v18 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-08  5:05                                   ` [PATCH v18 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-08  5:05                                   ` [PATCH v18 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget
2021-12-08  6:22                                     ` Junio C Hamano
2021-12-08  6:46                                       ` Aleen 徐沛文
2021-12-08 11:32                                         ` Junio C Hamano
2021-12-09  7:25                                   ` [PATCH v19 0/3] am: support --empty=(die|drop|keep) option and --allow-empty option to handle " Aleen via GitGitGadget
2021-12-09  7:25                                     ` [PATCH v19 1/3] doc: git-format-patch: describe the option --always 徐沛文 (Aleen) via GitGitGadget
2021-12-09  9:28                                       ` Bagas Sanjaya
2021-12-10  1:26                                         ` Aleen 徐沛文
2021-12-10  6:50                                           ` Bagas Sanjaya
2021-12-11  9:22                                             ` Junio C Hamano
2021-12-09  7:25                                     ` [PATCH v19 2/3] am: support --empty=<option> to handle empty patches 徐沛文 (Aleen) via GitGitGadget
2021-12-09  7:25                                     ` [PATCH v19 3/3] am: support --allow-empty to record specific " 徐沛文 (Aleen) via GitGitGadget

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=526ec6c0.87.17d35f63231.Coremail.pwxu@coremail.cn \
    --to=pwxu@coremail.cn \
    --cc=aleen42@vip.qq.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=phillip.wood123@gmail.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 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.