Re: [PATCH v9 3/5] t4205, t6006, t7102: make functions more readable

From: Alexey Shumkin <alex.crezoff@gmail.com>
Date: Jul 5, 2013, 08:51 UTC
Message-ID: <20130705085112.GE32072@ashu.dyn1.rarus.ru>
In-Reply-To: <7vsizt8kh4.fsf@alter.siamese.dyndns.org>

On Fri, Jul 05, 2013 at 01:44:07AM -0700, Junio C Hamano wrote:

Show 21 quoted lines

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> >> Perhaps like this.
> >> 
> >>     Function 'test_format' has become harder to read after its
> >>     change in de6029a2 (pretty: Add failing tests: --format output
> >>     should honor logOutputEncoding, 2013-06-26).  Simplify it by
> >>     moving its "should we expect it to fail?" parameter to the end.
> > I'm not sure whether this "last parameter" is needed in that code as far as we
> > already removed expected to fail tests
> 
> Whatever.
> 
> The above is an example of justifying a more vague "simple" ("is
> better" is implied) with a concrete point (i.e. By moving that to
> the end, you removed the need to conditionally shift $@ in the
> function to simplify the codepath), based on my _guess_ of what you
> possibly meant to say, from reading your description that did not
> give much clue for me to guess why you thought the result was "more
> elegant".  If my guess missed what your true justification was,
> please replace it with the more correct one ;-)

Ok

Show 12 quoted lines

> 
> >> I cannot read why you think the updated commit_msg is "more pretty"
> >> in the message or in the patch.
> >> 
> >> > -commit_msg () {
> >> > -	# String "initial. initial" partly in German (translated with Google Translate),
> >> > +commit_msg() {
> >> 
> >> Style.  Have SP on both sides of () in a shell function definition.
> > Could you point me to the coding style guide, please?
> 
> Documentation/CodingGuidelines::

Oh! :) thank you

>  
>  - We prefer a space between the function name and the parentheses. The
>    opening "{" should also be on the same line.
>    E.g.: my_function () {

Aha

-- 
Alexey Shumkin