On Fri, 21 Jul 2017 12:58:55 +0200 Michal Privoznik <mprivozn@redhat.com> wrote:
On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
On Fri, 21 Jul 2017 10:12:38 +0200 Michal Privoznik <mprivozn@redhat.com> wrote:
On 07/20/2017 08:21 PM, Tomáš Golembiovský wrote:
The documentation string has to follow the definition of a constant in the enum. Otherwise, the HTML documentation will be generated incorrectly.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- include/libvirt/libvirt-domain.h | 62 ++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 45f939a8c..2f3162d0f 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -583,56 +583,56 @@ typedef virDomainInterfaceStatsStruct *virDomainInterfaceStatsPtr; * Memory Statistics Tags: */ typedef enum { - /* The total amount of data read from swap space (in kB). */ VIR_DOMAIN_MEMORY_STAT_SWAP_IN = 0, - /* The total amount of memory written out to swap space (in kB). */ + /* The total amount of data read from swap space (in kB). */ VIR_DOMAIN_MEMORY_STAT_SWAP_OUT = 1, + /* The total amount of memory written out to swap space (in kB). */
While this fixes generated HTML, it messes up the header file. So if somebody is looking directly into header file they might get confused. The problem is in our doc generator.
I agree that it's not ideal solution. But since it's already used in the header, e.g. in virDomainBlockJobType and virDomainEventShutdownDetailType,
This one actually looks okay. Did you perhaps mean virConnectDomainEventDiskChangeReason?
No, I really meant virDomainEventShutdownDetailType. But you're right about virConnectDomainEventDiskChangeReason too. I didn't look at other headers, but as far as libvirt-domain.h is concerned those three seem to be all. There are also several (5?) misplaced comments for the sentinels if you care about those too.
I assumed it's acceptable. And the overall readability is not that bad because the constant+doc pairs are separated with newline from one another.
Nope. It's not. I've sent a patch that fixes those two places (I've went through all of our public headers and found just those two though):
https://www.redhat.com/archives/libvir-list/2017-July/msg00871.html
I recall this being discussed somewhere recently (probably on the list?). The proper fix IMO is to fix the generator so that it accepts both:
That would be the best solution, but I'm too scared to look at the generator. That would be a job for somebody else.
Yeah. Our generator is not that great. I wish that we'd switch to something already out there so that we don't have to maintain our own generator.
Michal
-- Tomáš Golembiovský <tgolembi@redhat.com>