git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 1/5] pull: rename pull.rename to pull.mode

From
Felipe Contreras <felipe.contreras@gmail.com>
Date
Oct 12, 2013, 01:15 UTC
Message-ID
<CAMP44s2y0UZ9uS8xtG2WDD=k5pHSG+K+_WM2dj-DVaUDy4djdA@mail.gmail.com>
In-Reply-To
<20131012005035.GA27939@sigill.intra.peff.net>
On Fri, Oct 11, 2013 at 7:50 PM, Jeff King <peff@peff.net> wrote:
Show 25 quoted lines
> 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.
Show 26 quoted lines
> 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
Previous: Jeff KingNext: Jeff King
Message 12 of 24 in “Preparation for non-ff pulls by default”
  1. 0/5 Preparation for non-ff pulls by defaultFelipe Contreras, Sep 9, 2013
  2. 1/5 pull: rename pull.rename to pull.modeFelipe Contreras, Sep 9, 2013
  3. Richard HansenSep 9, 2013
  4. Felipe ContrerasSep 9, 2013
  5. Jeff KingSep 10, 2013
  6. Felipe ContrerasSep 10, 2013
  7. Jeff KingSep 10, 2013
  8. Matthieu MoySep 10, 2013
  9. Felipe ContrerasOct 11, 2013
  10. Richard HansenOct 12, 2013
  11. Jeff KingOct 12, 2013
  12. Felipe ContrerasOct 12, 2013
  13. Jeff KingOct 12, 2013
  14. Felipe ContrerasOct 12, 2013
  15. Richard HansenOct 12, 2013
  16. Felipe ContrerasOct 12, 2013
  17. Richard HansenSep 19, 2013
  18. Felipe ContrerasOct 11, 2013
  19. Junio C HamanoSep 10, 2013
  20. 2/5 pull: refactor $rebase variable into $modeFelipe Contreras, Sep 9, 2013
  21. 3/5 pull: add --merge optionFelipe Contreras, Sep 9, 2013
  22. 4/5 pull: add merge-ff-only optionFelipe Contreras, Sep 9, 2013
  23. 5/5 pull: add warning on non-ff mergesFelipe Contreras, Sep 9, 2013
  24. Felipe ContrerasSep 9, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.