From: Felipe Contreras <felipe.contreras@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
Richard Hansen <rhansen@bbn.com>,
git@vger.kernel.org, Andreas Krey <a.krey@gmx.de>,
John Keeping <john@keeping.me.uk>,
Philip Oakley <philipoakley@iee.org>,
"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH v3 1/5] pull: rename pull.rename to pull.mode
Date: Fri, 11 Oct 2013 20:15:46 -0500 [thread overview]
Message-ID: <CAMP44s2y0UZ9uS8xtG2WDD=k5pHSG+K+_WM2dj-DVaUDy4djdA@mail.gmail.com> (raw)
In-Reply-To: <20131012005035.GA27939@sigill.intra.peff.net>
On Fri, Oct 11, 2013 at 7:50 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 11, 2013 at 06:56:23PM -0500, Felipe Contreras wrote:
>
>> > >>> > These deprecation warning messages should be written to stderr, and
>> > >>> > should probably be prefixed with "WARNING: ".
>> > >>>
>> > >>> Is there any deprecation warning that works this way?
>> > >>
>> > >> The ones in C code typically use warning(), which will prefix with
>> > >> "warning:" and write to stderr. They do not use all-caps, though.
>> > >>
>> > >> Try "git log --grep=deprecate -Swarning" for some examples.
>> > >
>> > > I'm asking about the ones in shell, because this is a shell script.
>> >
>> > No user cares whether "git pull" is written in shell. shell Vs C is an
>> > implementation detail, stdout Vs stderr is user-visible.
>>
>> You are free to go ahead and implement 'warning ()' in git-sh-setup.sh, in the
>> meantime no shell script does that, and that's no reason to reject this patch
>> series.
>
> You are completely missing Matthieu's point that we attempt to be
> consistent in the format of messages, as well as where they are output,
> and from a user's perspective it does not matter what language the tool
> is implemented in.
If we truly did that, there should be a warning () function, like in C.
> Also, you are wrong that there are no other shell scripts that behave as
> Richard said:
>
> $ git grep '>&2' | grep -i deprecate
> contrib/completion/git-completion.bash: echo "WARNING: this script is deprecated, please see git-completion.zsh" 1>&2
> contrib/examples/git-resolve.sh:echo 'WARNING: This command is DEPRECATED and will be removed very soon.' >&2
> git-lost-found.sh:echo "WARNING: '$0' is deprecated in favor of 'git fsck --lost-found'" >&2
>
> Please, can we just squash in the patch below and stop talking about
> this?
>
> diff --git a/git-pull.sh b/git-pull.sh
> index a9cf7ac..9c4091c 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -58,8 +58,8 @@ then
> if test "$rebase" = 'true'
> then
> mode="rebase"
> - echo "The configurations pull.rebase and branch.<name>.rebase are deprecated."
> - echo "Please use pull.mode and branch.<name>.pullmode instead."
> + echo >&2 "warning: The configurations pull.rebase and branch.<name>.rebase are deprecated."
> + echo >&2 "Please use pull.mode and branch.<name>.pullmode instead."
> fi
> fi
> test -z "$mode" && mode=merge
Are you sure you want me to squash that in? Because the warnings
wouldn't be consistent. Some would be "WARNING: " and others would be
"warning: ". Personally I don't care, but if your argument is
consistency, you should. If we had a warning () function, we could
truly be consistent.
--
Felipe Contreras
next prev parent reply other threads:[~2013-10-12 1:15 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-09 1:23 [PATCH v3 0/5] Preparation for non-ff pulls by default Felipe Contreras
2013-09-09 1:23 ` [PATCH v3 1/5] pull: rename pull.rename to pull.mode Felipe Contreras
2013-09-09 21:23 ` Richard Hansen
2013-09-09 22:49 ` Felipe Contreras
2013-09-10 2:21 ` Jeff King
2013-09-10 6:46 ` Felipe Contreras
2013-09-10 6:52 ` Jeff King
2013-09-10 8:16 ` Matthieu Moy
2013-10-11 23:56 ` Felipe Contreras
2013-10-12 0:32 ` Richard Hansen
2013-10-12 0:50 ` Jeff King
2013-10-12 1:15 ` Felipe Contreras [this message]
2013-10-12 1:25 ` Jeff King
2013-10-12 2:08 ` Felipe Contreras
2013-10-12 4:40 ` Richard Hansen
2013-10-12 6:21 ` Felipe Contreras
2013-09-19 19:33 ` Richard Hansen
2013-10-11 23:54 ` Felipe Contreras
2013-09-10 21:34 ` Junio C Hamano
2013-09-09 1:23 ` [PATCH v3 2/5] pull: refactor $rebase variable into $mode Felipe Contreras
2013-09-09 1:23 ` [PATCH v3 3/5] pull: add --merge option Felipe Contreras
2013-09-09 1:23 ` [PATCH v3 4/5] pull: add merge-ff-only option Felipe Contreras
2013-09-09 1:23 ` [PATCH v3 5/5] pull: add warning on non-ff merges Felipe Contreras
2013-09-09 1:49 ` Felipe Contreras
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='CAMP44s2y0UZ9uS8xtG2WDD=k5pHSG+K+_WM2dj-DVaUDy4djdA@mail.gmail.com' \
--to=felipe.contreras@gmail.com \
--cc=Matthieu.Moy@grenoble-inp.fr \
--cc=a.krey@gmx.de \
--cc=git@vger.kernel.org \
--cc=john@keeping.me.uk \
--cc=peff@peff.net \
--cc=philipoakley@iee.org \
--cc=rhansen@bbn.com \
--cc=sandals@crustytoothpaste.net \
/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).