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

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

 



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.


ACK, virBufferEscapeString nicely simplifies the code :)

Pushed.

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]