[libvirt] [PATCHv2 07/13] snapshot: simplify indentation of sysinfo

Hai Dong Li haidongl at linux.vnet.ibm.com
Fri Oct 21 07:29:55 UTC 2011


On 10/21/2011 06:41 AM, Eric Blake wrote:
> On 10/20/2011 04:20 AM, Daniel P. Berrange wrote:
>> On Thu, Oct 20, 2011 at 12:14:57PM +0200, Peter Krempa wrote:
>>> On 09/29/2011 06:22 PM, Eric Blake wrote:
>>>> The improvements to virBuffer, along with a paradigm shift to pass
>>>> the original buffer through rather than creating a second buffer,
>>>> allow us to shave off quite a few lines of code.
>>>>
>
>>>>
>>> I'd squash in the attached patch, but it's not necessary as it gets
>>> rid of non automatic indentation whitespace, but makes the code look
>>> cleaner :)
>>
>> I'm not entirely convinced this is a good idea.  This means that
>> when looking at the code, it is no longer obvious what the nesting
>> of XML elements is supposed to be - they are all the level.
>
> I concur with Daniel here - I debated about dropping the leading 
> whitespace and using a lot more virBufferAdjustIndent when first 
> writing this series (since it would have been less work to convert 
> from my v1, where I had already removed all the leading whitespace 
> anyways), but it just doesn't scale as well.  My end decision was to 
> only remove whitespace and start at level 0 just for the functions 
> that can be called from multiple locations; single-use call-chains 
> were easier to keep indented as they were before the series.
>
> The only reason I used virBufferAdjustIndent inside 
> virSysinfoProcessorFormat, against the rule of thumb that I generally 
> used of keeping the in-function indentation, was to fit things in 80 
> columns.
>

I agree with you and that's a reasonable reason.:-)

>>
>> ACK, virBufferEscapeString nicely simplifies the code :)
>
> Pushed.
>




More information about the libvir-list mailing list