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
Jeff King <peff@peff.net>
Date
Oct 12, 2013, 01:25 UTC
Message-ID
<20131012012515.GA1778@sigill.intra.peff.net>
In-Reply-To
<CAMP44s2y0UZ9uS8xtG2WDD=k5pHSG+K+_WM2dj-DVaUDy4djdA@mail.gmail.com>
On Fri, Oct 11, 2013 at 08:15:46PM -0500, Felipe Contreras wrote:
Show 10 quoted lines
> >> 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.

Or people could hand-code them to look similar, which is exactly what has happened.

If you want to factor out a warning function to clean up the code, be my guest. But the lack of one does not provide an argument that you should break consistency.

Show 11 quoted lines
> > -               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."
> [...]
> 
> 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.

It is significant in the most important ways, which are labeling it at all, and sending it to stderr. Capitalization is less important, in my opinion.

Using a lowercase version is much more consistent with the warnings produced by C code, which is why I chose it over the capitalized version. Again, if you want to change the existing WARNING cases in the shell scripts to be consistent with C output, be my guest.

Do you actually have some reason for wanting to output to go to stdout?
-Peff
Previous: Felipe ContrerasNext: Felipe Contreras
Message 13 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.