[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/11] Add operations to qlist to allow it to be
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 02/11] Add operations to qlist to allow it to be used as a stack |
Date: |
Fri, 13 Nov 2009 09:51:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.4pre) Gecko/20091014 Fedora/3.0-2.8.b4.fc11 Thunderbird/3.0b4 |
Am 12.11.2009 18:13, schrieb Anthony Liguori:
> Kevin Wolf wrote:
>> Unfortunately. There are places where such comments could be a good
>> specification on what an interface is actually meant to work like
>> (particularly in error cases). Currently you often can't tell if the
>> implementation or the caller of a function is buggy.
>>
>> Not sure if they are really useful for the simple qlist.c functions (but
>> even there the function name does not tell me what it's doing with NULL
>> parameters), but it might be helpful to have a general discussion about
>> it. I think in general qemu is poorly commented.
>>
>
> I agree, but I don't think the solution is forcing boiler plate
> commenting styles. I think what we could improve on is asking people to
> comment bits of code during review.
Be sure that I'll be asking for comments in qcow2 patches. I did in the
past and I'll continue to do so. ;-)
I agree that boiler plates are not going to help per se. But I think
what actually does play a role in commenting is what surrounding code
looks like and it's also habits. And it's probably not wrong to say that
we're currently in a habit of not documenting anything. If boiler plates
in some places are the price to get into a habit of documenting things,
I think considering to accept that price wouldn't be completely
unreasonable.
Another thing is that we could ask more often to move explanations from
mails into the code. The explanation often exists and sometimes we're
lucky enough that it ends up at least in the commit log, but there are
cases where it's in PATCH 0/n or buried in the corresponding mail thread.
Kevin
- [Qemu-devel] [PATCH 11/11] Add a unit test for JSON support, (continued)
- [Qemu-devel] [PATCH 11/11] Add a unit test for JSON support, Anthony Liguori, 2009/11/11
- [Qemu-devel] [PATCH 06/11] Add a QBool type, Anthony Liguori, 2009/11/11
- [Qemu-devel] [PATCH 03/11] Allow strings to grow in size, Anthony Liguori, 2009/11/11
- [Qemu-devel] [PATCH 08/11] Add a JSON message boundary identifier, Anthony Liguori, 2009/11/11
- [Qemu-devel] [PATCH 02/11] Add operations to qlist to allow it to be used as a stack, Anthony Liguori, 2009/11/11
[Qemu-devel] [PATCH 04/11] Add a QFloat datatype, Anthony Liguori, 2009/11/11
[Qemu-devel] [PATCH 10/11] Add a QObject JSON wrapper, Anthony Liguori, 2009/11/11
[Qemu-devel] [PATCH 07/11] Add a lexer for JSON, Anthony Liguori, 2009/11/11