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

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

 



Alexey Shumkin <alex.crezoff@xxxxxxxxx> 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 ;-)

>> 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::

 - We prefer a space between the function name and the parentheses. The
   opening "{" should also be on the same line.
   E.g.: my_function () {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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]