On Fri, 21 Jul 2017 12:58:55 +0200
Michal Privoznik <mprivozn@xxxxxxxxxx> wrote:
> On 07/21/2017 12:25 PM, Tomáš Golembiovský wrote:
> > On Fri, 21 Jul 2017 10:12:38 +0200
> > Michal Privoznik <mprivozn@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>> ---
> >>> 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@xxxxxxxxxx>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
