[libvirt] [PATCH] Generate HACKING from docs/hacking.html.in

Eric Blake eblake at redhat.com
Fri Nov 12 17:38:09 UTC 2010


On 11/12/2010 10:15 AM, Matthias Bolte wrote:
>>>  HACKING              |  602 +++++++++++++++++++++++++++++---------------------
>>
>> A lot of this diff is whitespace; the rest looks like it is pulling in
>> missed bits.  Overall, it looks pretty decent; and it's certainly more
>> maintainable!
>>

Patch 1 review:

> Tweak pre tags to achieve proper indentation of their
> plaintext representation. Also use more b/i/code tags.
> ---
>  docs/hacking.html.in |  351 +++++++++++++++++++++++++++-----------------------
>  1 files changed, 189 insertions(+), 162 deletions(-)
> 
> @@ -336,57 +358,57 @@
>      </p>
>  
>      <ul>
> -      <li><p>eg to allocate  a single object:</p>
> -
> +      <li><p>e.g. to allocate  a single object:</p>

Using "e.g." in every <li> of this bullet-list seems redundant; but
cleaning that up can be a separate patch (in fact, I've already written
the patch, as part of the VIR_RESIZE series that I'm trying to revive,
but haven't pushed it to the list yet until yours is in, so I'll get
more merge conflicts if you change wording now :)

>        <li><p>eg close a file descriptor in an error path, without losing
>               the previous errno value</p>

Should errno be surrounded in <code>, since it is a variable name?

> +<pre>
>    char *
> -  somefunction(...) {
> +  somefunction(...)
> +  {

I like this change...

> @@ -569,13 +595,13 @@
>    #include "util.h"               Any libvirt internal header files.
>    #include "buf.h"
>  
> -  static myInternalFunc ()        The actual code.
> +  static myInternalFunc()         The actual code.
>    {

and this is already wrong, since it lacks a return type.  Let's adjust
it to match the same style:

static int
myInternalFunc()
{

there are several code analysis tools that work better if all function
implementation names start on column 0, with return type and other
decorations on the previous line.  Even though we don't yet have a
consistent style throughout libvirt, we might as well have a single
style in the hacking guide.

ACK, either as-is, or if you want to make a few tweaks per my above
comments.  Either way, it's a good cleanup and documentation-only, so I
don't need to see a v2 before you push this patch.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101112/35b27799/attachment-0001.sig>


More information about the libvir-list mailing list