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

[Programmingkid]

On Jan 21, 2015, at 2:54 AM, Markus Armbruster wrote:

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

<Puts up white flag> I will indent my code at around the 60 to 70
character size.