[libcamera-devel] [PATCH] libcamera: Move uneeded comment from pipeline handler

Laurent Pinchart laurent.pinchart at ideasonboard.com
Tue Jun 29 15:24:54 CEST 2021


On Tue, Jun 29, 2021 at 01:22:41PM +0100, Kieran Bingham wrote:
> Hi JM,
> 
> On 28/06/2021 21:04, Jean-Michel Hautbois wrote:
> > The comment is a implementation detail and does not belong to API
> > documentation. Move it inside the function.
> > 
> 
> Sounds reasonable indeed.
> 
> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> > ---
> >  src/libcamera/pipeline_handler.cpp | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index f626eddd..39eed678 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -707,14 +707,14 @@ void PipelineHandlerFactory::registerType(PipelineHandlerFactory *factory)
> >  
> >  /**
> >   * \brief Retrieve the list of all pipeline handler factories
> > - *
> > - * The static factories map is defined inside the function to ensures it gets
> > - * initialized on first use, without any dependency on link order.
> > - *
> >   * \return the list of pipeline handler factories
> >   */
> >  std::vector<PipelineHandlerFactory *> &PipelineHandlerFactory::factories()
> >  {
> > +	/* The static factories map is defined inside the function to ensures
> 
> Though a multiline comment should have /* on it's own
> 
>  /*
>   * line 1.
>   * line 2.
>   */
> 
> I'd say the s/ensures/ensure/ could be fixed up in this patch too, with
> just a comment in the commit message to say, it also fixes a grammatical
> error in the comment.

And the subject line should read

libcamera: pipeline_handler: Hide implementation detail comment from doxygen

or something similar.

> With those:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> > +	 * it gets initialized on first use, without any dependency on
> > +	 * link order.
> > +	 */
> >  	static std::vector<PipelineHandlerFactory *> factories;
> >  	return factories;
> >  }

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list