[libcamera-devel,2/4] Documentation: coding-style: Document order of includes

Message ID 20191023135258.32256-2-laurent.pinchart@ideasonboard.com
State Accepted
Commit 73a11cbf78342ecc875709e92abacee165530823
Headers show
Series
  • [libcamera-devel,1/4] Documentation: coding-style: Document usage of C compatibility headers
Related show

Commit Message

Laurent Pinchart Oct. 23, 2019, 1:52 p.m. UTC
We follow the Google C++ Style Guide rule on include ordering with a few
tweaks. Document our rule explicitly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/coding-style.rst | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Kieran Bingham Oct. 23, 2019, 2:02 p.m. UTC | #1
Hi Laurent,

On 23/10/2019 14:52, Laurent Pinchart wrote:
> We follow the Google C++ Style Guide rule on include ordering with a few
> tweaks. Document our rule explicitly.
> 

Happy to see this documented for clarity.

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Only white-space comments below.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  Documentation/coding-style.rst | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> index ced3155169a6..c53248234c34 100644
> --- a/Documentation/coding-style.rst
> +++ b/Documentation/coding-style.rst
> @@ -61,6 +61,29 @@ document:
>    Code Style for indentation, braces, spacing, etc
>  * Header guards are formatted as '__LIBCAMERA_FILE_NAME_H__'
>  

Single separating line here,

> +Order of Includes
> +~~~~~~~~~~~~~~~~~
> +
> +Headers shall be included at the beginning of .c, .cpp and .h files, right
> +after the file description comment block and, for .h files, the header guard
> +macro. For .cpp files, if the file implements an API declared in a header file,
> +that header file shall be included first in order to ensure it is
> +self-contained.
> +
> +The headers shall be grouped and ordered as follows.
> +
> + # The header declaring the API being implemented (if any)
> + # The C and C++ system and standard library headers
> + # Other libraries' headers, with one group per library
> + # Other project's headers
> +
> +Groups of headers shall be separated by a single blank line. Headers within
> +each group shall be sorted alphabetically.
> +
> +System and library headers shall be included with angle brackets. Project
> +headers shall be included with angle brackets for the libcamera public API
> +headers, and with double quotes for other libcamera headers.
> +
>  

But double here, just like the previous patch, so the adding an extra at
the bottom is consistent between the two making it look intentional -
but /both/ patches have a single separating line at the top or the entry?


>  C++ Specific Rules
>  ------------------
>
Laurent Pinchart Oct. 23, 2019, 2:06 p.m. UTC | #2
Hi Kieran,

On Wed, Oct 23, 2019 at 03:02:16PM +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 23/10/2019 14:52, Laurent Pinchart wrote:
> > We follow the Google C++ Style Guide rule on include ordering with a few
> > tweaks. Document our rule explicitly.
> > 
> 
> Happy to see this documented for clarity.
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Only white-space comments below.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  Documentation/coding-style.rst | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
> > index ced3155169a6..c53248234c34 100644
> > --- a/Documentation/coding-style.rst
> > +++ b/Documentation/coding-style.rst
> > @@ -61,6 +61,29 @@ document:
> >    Code Style for indentation, braces, spacing, etc
> >  * Header guards are formatted as '__LIBCAMERA_FILE_NAME_H__'
> >  
> 
> Single separating line here,
> 
> > +Order of Includes
> > +~~~~~~~~~~~~~~~~~
> > +
> > +Headers shall be included at the beginning of .c, .cpp and .h files, right
> > +after the file description comment block and, for .h files, the header guard
> > +macro. For .cpp files, if the file implements an API declared in a header file,
> > +that header file shall be included first in order to ensure it is
> > +self-contained.
> > +
> > +The headers shall be grouped and ordered as follows.
> > +
> > + # The header declaring the API being implemented (if any)
> > + # The C and C++ system and standard library headers
> > + # Other libraries' headers, with one group per library
> > + # Other project's headers
> > +
> > +Groups of headers shall be separated by a single blank line. Headers within
> > +each group shall be sorted alphabetically.
> > +
> > +System and library headers shall be included with angle brackets. Project
> > +headers shall be included with angle brackets for the libcamera public API
> > +headers, and with double quotes for other libcamera headers.
> > +
> 
> But double here, just like the previous patch, so the adding an extra at
> the bottom is consistent between the two making it look intentional -
> but /both/ patches have a single separating line at the top or the entry?

There's a single line before the ~~~ headers, but a double line before
the --- headers. That's to stay consistent with the current style of the
file, I don't really have a preference.

> >  C++ Specific Rules
> >  ------------------
> >

Patch

diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst
index ced3155169a6..c53248234c34 100644
--- a/Documentation/coding-style.rst
+++ b/Documentation/coding-style.rst
@@ -61,6 +61,29 @@  document:
   Code Style for indentation, braces, spacing, etc
 * Header guards are formatted as '__LIBCAMERA_FILE_NAME_H__'
 
+Order of Includes
+~~~~~~~~~~~~~~~~~
+
+Headers shall be included at the beginning of .c, .cpp and .h files, right
+after the file description comment block and, for .h files, the header guard
+macro. For .cpp files, if the file implements an API declared in a header file,
+that header file shall be included first in order to ensure it is
+self-contained.
+
+The headers shall be grouped and ordered as follows.
+
+ # The header declaring the API being implemented (if any)
+ # The C and C++ system and standard library headers
+ # Other libraries' headers, with one group per library
+ # Other project's headers
+
+Groups of headers shall be separated by a single blank line. Headers within
+each group shall be sorted alphabetically.
+
+System and library headers shall be included with angle brackets. Project
+headers shall be included with angle brackets for the libcamera public API
+headers, and with double quotes for other libcamera headers.
+
 
 C++ Specific Rules
 ------------------