Re: [PATCH v7 9/9] docs/doxygen: doxygen documentation for grant_table.h

[Luca Fancellu <luca.fancellu@xxxxxxx>]

Hi Jan,

> On 5 Jul 2021, at 14:03, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 05.07.2021 12:51, Luca Fancellu wrote:
>> Modification to include/public/grant_table.h:
>> 
>> 1) Add doxygen tags to:
>> - Create Grant tables section
>> - include variables in the generated documentation
>> - Used @keepindent/@endkeepindent to enclose comment
>>   section that are indented using spaces, to keep
>>   the indentation.
>> 2) Add .rst file for grant table
>> 
>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>> ---
>> v7 changes:
>> - commit message changed
>> - Add comment about grant table queries and uses
>> to the documentation
>> v6 changes:
>> - Fix misaligned comment
>> - Moved comments to make them display in the docs
>> - Included more documentation in the docs
>> v5 changes:
>> - Move GNTCOPY_* define next to the flags field
>> v4 changes:
>> - Used @keepindent/@endkeepindent doxygen commands
>>  to keep text with spaces indentation.
>> - drop changes to grant_entry_v1 comment, it will
>>  be changed and included in the docs in a future patch
>> - Move docs .rst to "common" folder
>> v3 changes:
>> - removed tags to skip anonymous union/struct
>> - moved back comment pointed out by Jan
>> - moved down defines related to struct gnttab_copy
>>  as pointed out by Jan
>> v2 changes:
>> - Revert back to anonymous union/struct
>> - add doxygen tags to skip anonymous union/struct
>> ---
>> docs/hypercall-interfaces/arm64.rst           |   1 +
>> .../common/grant_tables.rst                   |   9 +
> 
> In patch 8 you now add arm32.rst and x86.rst as well, so it's at
> least odd that here you alter only one of the three. However, ...
> 
>> --- a/docs/hypercall-interfaces/arm64.rst
>> +++ b/docs/hypercall-interfaces/arm64.rst
>> @@ -8,6 +8,7 @@ Starting points
>> .. toctree::
>>    :maxdepth: 2
>> 
>> +   common/grant_tables
> 
> ... to me this seems the wrong way round anyway: I'd rather see
> common stuff not be linked from per-arch locations, but per-arch
> docs to be down the hierarchy from common ones.

The things is that common stuff is not really common, if you go and solve every
define and so on, you end up with a different content for x86, aarch64, arm.

So you can’t have a common.rst containing the common things because depending 
on each
platform the content will change.

Thanks anyway for your feedback, I think it’s good to make the community know 
these changing
and discuss them all together.

Cheers,

Luca

> 
> Jan