Re: [PATCH v4 2/7] Documentation: add shell guidelines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 5, 2018 at 9:48 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Matthew DeVore <matvore@xxxxxxxxxx> writes:
>
> > Add the following guideline to Documentation/CodingGuidelines:
> >
> >       &&, ||, and | should appear at the end of lines, not the
> >       beginning, and the \ line continuation character should be
> >       omitted
>
> "should be omitted" sounds as if it is the norm to have such a
> character, but it is not.  The text in the actual patch body does a
> much better job than this.
>
> Perhaps
>
>         Break overlong lines after "&&", "||", and "|", not before
>         them; that way the command can continue to subsequent lines
>         without backslash at the end.
That sounds good. Fixed.

> > + - If a command sequence joined with && or || or | spans multiple
> > +   lines, put each command on a separate line and put && and || and |
> > +   operators at the end of each line, rather than the start. This
> > +   means you don't need to use \ to join lines, since the above
> > +   operators imply the sequence isn't finished.
>
> Correct.  Even though I wonder if we need to say the last sentence,
> which is rather obvious, patch is already written and I do not see
> much point editing this further.
ack

> > @@ -466,6 +466,34 @@ And here are the "don'ts:"
> >     platform commands; just use '! cmd'.  We are not in the business
> >     of verifying that the world given to us sanely works.
> >
> > + - Don't use Git upstream in the non-final position in a piped chain, as
> > +   in:
>
> "upstream in the non-final position" is a bit redundant, isn't it?
>
>   - Don't feed the output of 'git' to a pipe, as in:

Done, but I changed it to "Don't feed the output of a git command to a
pipe, as in:" since referring to it as "git command" without the
quotes seems rather common in this file.

Thank you for the review!



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux