Re: [PATCH v13 01/25] v4l: fwnode: Move KernelDoc documentation to the header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/19/17 12:48, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Friday, 15 September 2017 17:17:00 EEST Sakari Ailus wrote:
>> In V4L2 the practice is to have the KernelDoc documentation in the header
>> and not in .c source code files. This consequently makes the V4L2 fwnode
>> function documentation part of the Media documentation build.
>>
>> Also correct the link related function and argument naming in
>> documentation.
>>
>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
>> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> Acked-by: Pavel Machek <pavel@xxxxxx>
> 
> I'm still very opposed to this. In addition to increasing the risk of 
> documentation becoming stale, it also makes review more difficult. I'm 
> reviewing patch 05/25 of this series and I have to jump around the patch to 
> verify that the documentation matches the implementation, it's really 
> annoying.
> 
> We should instead move all function documentation from header files to source 
> files.

I disagree with this. Yes, it makes reviewing harder, but when you have to
*use* these functions as e.g. a driver developer, then having it in the
header is much more convenient.

Regards,

	Hans

> 
>> ---
>>  drivers/media/v4l2-core/v4l2-fwnode.c | 75 --------------------------------
>> include/media/v4l2-fwnode.h            | 81 +++++++++++++++++++++++++++++++-
>> 2 files changed, 80 insertions(+), 76 deletions(-)
> 
> [snip]
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux