Re: [Qemu-devel] [PATCH v8] block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD

[Markus Armbruster]

Programmingkid <programmingk...@gmail.com> writes:

> On Jan 20, 2015, at 3:28 PM, Markus Armbruster wrote:
>
>> Programmingkid <programmingk...@gmail.com> writes:
>> 
>>> On Jan 20, 2015, at 10:22 AM, Eric Blake wrote:
>>> 
>>>> On 01/20/2015 07:29 AM, Programmingkid wrote:
>>>>> 
>>>>> On Jan 20, 2015, at 3:33 AM, Markus Armbruster wrote:
>>>>> 
>>>>>> Programmingkid <programmingk...@gmail.com> writes:
>>>>>> 
>>>>>>> Subject was: 
>>>>>>> Re: [PATCH v7] block/raw-posix.c: Fixes raw_getlength() 
>>>>>>> on Mac OS X so that it reports the correct length of a real CD
>>>>>> 
>>>>>> Patch history information goes...
>>>> 
>>>>>> 
>>>>>> ... below the --- divider.
>>>>> 
>>>>> I thought I did this. The information above is the description of
>>>>> the patch.
>>>>> Not its history.
>>>> 
>>>> Anything that mentions 'v7' is history.  When you read 'git log', you
>>>> will not see mentions of 'v7', because no one cares how many tries it
>>>> took to get a patch into git.  Knowing about v7 only matters to the
>>>> reviewers of v8, hence it is patch history that belongs after the divider.
>>> 
>>> Ok. 
>>> 
>>>> 
>>>> 
>>>>>>> +
>>>>>>> +            if (ioctl(fd, DKIOCGETBLOCKCOUNT, &sectors) == 0
>>>>>>> +               && ioctl(fd, DKIOCGETBLOCKSIZE, &sector_size) == 0) {
>>>> 
>>>> Indentation looks off here.
>>> 
>>> It does look a little odd, but it also communicates that this is one
>>> statement (IMHO).
>> 
>> It's not how the rest of QEMU is indented.  Please try to blend in :)
>> 
>> I feel bad about notpicking v8 of an obviously useful and patch that is
>> basically just fine except for these little things.  Thanks for
>> persevering!
>
> Thanks for your encouragement. I personally would have had that whole if
> condition on one line, but others want an 80 line maximum. 

Humans tend to have trouble following long lines with their eyes (I sure
do).  Typographic manuals suggest to limit columns to roughly 60
characters for exactly that reason[*].  Code can be a bit wider since
it's commonly indented[**].

For commit messages, 70 characters is already a compromise between
legibility and "writability".


[*] https://en.wikipedia.org/wiki/Column_(typography)#Typographic_style

[**] Deep nesting is no excuse for exceeding the width limit, because
deep nesting is no excuse for *anything* :)