git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "SZEDER Gábor" <szeder@ira.uka.de>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Teemu Likonen <tlikonen@iki.fi>
Subject: Re: [PATCH 2/6] merge, pull: introduce '--(no-)stat' option
Date: Sat, 05 Apr 2008 19:36:57 -0700	[thread overview]
Message-ID: <7vzls7so8m.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 1207445027-3152-3-git-send-email-szeder@ira.uka.de

SZEDER Gábor <szeder@ira.uka.de> writes:

> '--(no-)summary' options are still accepted, but are not advertised.

Given that...

> diff --git a/git-merge.sh b/git-merge.sh
> index 7dbbb1d..1b693ad 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -8,8 +8,8 @@ OPTIONS_SPEC="\
>  git-merge [options] <remote>...
>  git-merge [options] <msg> HEAD <remote>
>  --
> -summary              show a diffstat at the end of the merge
> -n,no-summary         don't show a diffstat at the end of the merge
> +stat                 show a diffstat at the end of the merge
> +n,no-stat            don't show a diffstat at the end of the merge
>  squash               create a single commit instead of doing a merge
>  commit               perform a commit if the merge sucesses (default)
>  ff                   allow fast forward (default)

...this hunk removes the original options from OPTIONS_SPEC, I suspect you
would get "unknown option 'stat'" from underlying "git rev-parse --parseopt".

You probably haven't noticed the breakage because you replaced all
existing --summary with --stat in the tests, though.  Oops.

We would want to take a three-stage approach where we (1) start accepting
both forms without changing the official names shown to the users, (2)
deprecate the old names and make the new ones official, and then (3)
finally remove the old ones.  Your 2 thru 5 roll (1) and (2) into one
step.

I would not have major problem with this "hasty deprecation" in "usage"
strings, but I find it somewhat problematic to stop mentioning the old
names in the documentation and bash completion from day one.  People learn
old names elsewhere (e.g. in somebody's blog entry) and then try to find
the description in their manual and they are already removed from your
copy.  Oops.

So I pretty much prefer to have an explicit deprecation period where both
forms are not just accepted but described as equals, but with the older
ones marked clearly as deprecated and planned for removal, so that people
would know what is going on.

Other than that, the rest of the series looked Ok from my cursory review.

  parent reply	other threads:[~2008-04-06  2:38 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-03  9:03 [PATCH 0/2] merge --summary vs. merge.summary SZEDER Gábor
2008-04-03  9:03 ` [PATCH 1/2] merge, pull: rename '--summary' option to '--diffstat' SZEDER Gábor
2008-04-03  9:03   ` [PATCH 2/2] merge, pull: add option to put summary into the merge commit message SZEDER Gábor
2008-04-03 10:30 ` [PATCH 0/2] merge --summary vs. merge.summary Jeff King
2008-04-03 11:03   ` SZEDER Gábor
2008-04-05 14:48   ` [PATCH] merge, pull: introduce '--diffstat' option SZEDER Gábor
2008-04-05 15:35     ` Teemu Likonen
2008-04-05 18:51       ` Junio C Hamano
2008-04-06  1:23         ` [PATCH 0/6] merge summary and diffstat options cleanup SZEDER Gábor
2008-04-06  1:23           ` [PATCH 1/6] doc: moved merge.* config variables into separate merge-config.txt SZEDER Gábor
2008-04-06  1:23             ` [PATCH 2/6] merge, pull: introduce '--(no-)stat' option SZEDER Gábor
2008-04-06  1:23               ` [PATCH 3/6] add 'merge.stat' config variable SZEDER Gábor
2008-04-06  1:23                 ` [PATCH 4/6] fmt-merge-msg: add '--(no-)log' options and 'merge.log' " SZEDER Gábor
2008-04-06  1:23                   ` [PATCH 5/6] merge, pull: add '--(no-)log' command line option SZEDER Gábor
2008-04-06  1:23                     ` [PATCH 6/6] merge: remove deprecated summary and diffstat options and config variables SZEDER Gábor
2008-04-06  2:36               ` Junio C Hamano [this message]
2008-04-16  0:38                 ` [PATCH v2 00/12] merge summary and diffstat options cleanup SZEDER Gábor
2008-04-16  0:39                   ` [PATCH v2 01/12] doc: moved merge.* config variables into separate merge-config.txt SZEDER Gábor
2008-04-16  0:39                     ` [PATCH v2 02/12] merge, pull: introduce '--(no-)stat' options SZEDER Gábor
2008-04-16  0:39                       ` [PATCH v2 03/12] add 'merge.stat' config variable SZEDER Gábor
2008-04-16  0:39                         ` [PATCH v2 04/12] t6200-fmt-merge-msg: put expected messages into different files SZEDER Gábor
2008-04-16  0:39                           ` [PATCH v2 05/12] fmt-merge-msg: add '--(no-)log' options and 'merge.log' config variable SZEDER Gábor
2008-04-16  0:39                             ` [PATCH v2 06/12] merge, pull: add '--(no-)log' command line option SZEDER Gábor
2008-04-16  0:39                               ` [PATCH v2 07/12] merge, pull: mark '--(no-)summary' options as deprecated SZEDER Gábor
2008-04-16  0:39                                 ` [PATCH v2 08/12] merge, pull: mark the config variable 'merge.diffstat' " SZEDER Gábor
2008-04-16  0:39                                   ` [PATCH v2 09/12] fmt-merge-msg: mark summary-related option and config variable " SZEDER Gábor
2008-04-16  0:39                                     ` [PATCH v2 10/12] merge, pull: remove deprecated '--(no-)summary' options SZEDER Gábor
2008-04-16  0:39                                       ` [PATCH v2 11/12] merge, pull: remove deprecated 'merge.diffstat' config variable SZEDER Gábor
2008-04-16  0:39                                         ` [PATCH v2 12/12] fmt-merge-msg: remove deprecated summary-related options and " SZEDER Gábor
2008-04-16  4:23                   ` [PATCH v2 00/12] merge summary and diffstat options cleanup Junio C Hamano
2008-04-06 13:53         ` [PATCH] merge, pull: introduce '--diffstat' option Jeff King
2008-04-06 14:37           ` Jakub Narebski

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=7vzls7so8m.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=szeder@ira.uka.de \
    --cc=tlikonen@iki.fi \
    /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).