Re: [PATCH 1/9] conf: eliminate hardcoded indent from domain xml

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

 



On 03/06/2014 07:15 PM, Eric Blake wrote:
> On 03/06/2014 08:24 AM, Laine Stump wrote:
>>      if (def->label) {
>>          virBufferAddLit(buf, ">\n");
>> -        virBufferEscapeString(buf, "  <label>%s</label>\n",
>> +        virBufferAdjustIndent(buf, 2);
>> +        virBufferEscapeString(buf, "<label>%s</label>\n",
>>                                def->label);
>> +        virBufferAdjustIndent(buf, -2);
>>          virBufferAddLit(buf, "</seclabel>\n");
> For small hunks like this where all the context is local, this just adds
> lines of code.  I'm almost of the opinion that it's easier to have each
> function start with no indent, and any strings it prints locally are
> correctly indented (allowing leading whitespace), while any helper
> functions it calls are surrounded by AdjustIndent (since the helper
> function also prints at a local top level).

Yeah, I thought of that too, but it made it easier to verify, as you say
below, and once I got going it was difficult to pick a cutoff point, so
I just did it 100%.

> The clincher is that our testsuite validates that we didn't break any
> output :) ACK. 

Definitely I wouldn't have been able to make these changes with any
level of confidence without the tests (and yes, they did catch a few
things I'd missed, e.g. accidentally adding indent instead of removing).

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