Re: [PATCH] media: Document coding style requirements

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

 



On Thu, Oct 21, 2021 at 04:00:40PM +0200, Hans Verkuil wrote:
> On 13/10/2021 11:20, Jacopo Mondi wrote:
> > There are a few additional coding style conventions in place in
> > the media subsystem. If they do not get documented, it's hard to enforce
> > them during review as well as it is hard for developers to follow them
> > without having previously contributed to the subsystem.
> > 
> > Add them to the subsystem profile documentation.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx>
> > ---
> > 
> > All points are up for discussion ofc.
> > 
> > But the idea is to get to have more requirement defined, as otherwise
> > it's very hard to enforce them during review.
> > 
> > Thanks
> >    j
> > 
> > ---
> >  .../media/maintainer-entry-profile.rst        | 24 +++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/Documentation/driver-api/media/maintainer-entry-profile.rst b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > index eb1cdfd280ba..9c376f843e1c 100644
> > --- a/Documentation/driver-api/media/maintainer-entry-profile.rst
> > +++ b/Documentation/driver-api/media/maintainer-entry-profile.rst
> > @@ -180,6 +180,30 @@ In particular, we accept lines with more than 80 columns:
> >      - when they avoid a line to end with an open parenthesis or an open
> >        bracket.
> > 
> > +There are a few additional requirements which are not enforced by tooling
> > +but mostly during the review process:
> > +
> > +    - C++ style comments are not allowed, if not for SPDX headers;
> 
> if not -> except
> 
> > +    - hexadecimal values should be spelled using lowercase letters;
> > +    - one structure/enum member declaration per line;
> > +    - one variable declaration per line;
> 
> Hmm, I don't mind something like: int i, j;
> 
> But for anything more complex I too prefer one declaration per line.
> 
> > +    - prefer variable declaration order in reverse-x-mas-tree over
> > +      initialization at variable declare time;
> 
> Add something like:
> 
> ...unless there are dependencies or other readability reasons to
> depart from this.

This should probably go as the top-level, it's a valid comment for most
(all ?) rules.

> > +
> > +      As an example, the following style is preferred::
> > +
> > +         struct priv_struct *priv = container_of(....)
> > +         struct foo_struct *foo = priv->foo;
> > +         int b;
> > +
> > +         b = a_very_long_operation_name(foo, s->bar)
> > +
> > +      over the following one::
> > +
> > +         struct priv_struct *priv = container_of(....)
> > +         struct foo_struct *foo = priv->foo;
> > +         int b = a_very_long_operation_name(foo, s->bar)
> 
> I'm not sure if this is what you typically see.
> 
> Perhaps this is a better example:
> 
> 	int i;
> 	struct foo_struct *foo = priv->foo;
> 	int result;
> 
> should be written as:
> 
> 	struct foo_struct *foo = priv->foo;
> 	int result;
> 	int i;
> 
> > +
> >  Key Cycle Dates
> >  ---------------
> > 

-- 
Regards,

Laurent Pinchart



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux