openbmc coding standards validation in CI

openbmc coding standards validation in CI

[Andrew Geissler]

During the hackathon meetup, we decided to try and get something in
our CI jobs that would validate the code formatting and automatically
-1 the gerrit commit if it's not up to par.  The reasoning behind this
is we still have a lot of code review comments coming in this area,
which is wasteful for the reviewers.

Ed, Adriana, and I all worked different pieces of it.  We're using the
clang-format tool to format (and then validate the format).  The idea
is you do the initial run of the tool against your repo and check the
.clang-format file in with your changes.  The presence of the file
will cause the CI job to validate the code formatting.  After the
.clang-format is merged, CI jobs will fail if the new code doesn't
follow the standard.  The CI jenkins job will output the diff of the
code that is not conforming to the console.

We've tried our best to get the .clang-format file to match up with
our requirements in
https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md.

The .clang-format file and the changes it did to the sdbusplus repo
can be found in https://gerrit.openbmc-project.xyz/#/c/8461/.  Review
comments appreciated.

We've also got a similar commit coming that will validate python code.
That uses the new "pycodestyle --show-source ." tool and will be
triggered off of the presence of a setup.cfg file in the repo
(https://gerrit.openbmc-project.xyz/#/c/8462/).

Coding guidelines are always ... heated discussions ... so I think
there's some room for individual repo maintainers to tweak the
.clang-format a bit if needed.  In general we should all be conforming
to our cpp-style-and-conventions.md.

Andrew

Re: openbmc coding standards validation in CI

[Stewart Smith]

Andrew Geissler <geissonator@gmail.com> writes:
> During the hackathon meetup, we decided to try and get something in
> our CI jobs that would validate the code formatting and automatically
> -1 the gerrit commit if it's not up to par.  The reasoning behind this
> is we still have a lot of code review comments coming in this area,
> which is wasteful for the reviewers.
>
> Ed, Adriana, and I all worked different pieces of it.  We're using the
> clang-format tool to format (and then validate the format).  The idea
> is you do the initial run of the tool against your repo and check the
> .clang-format file in with your changes.  The presence of the file
> will cause the CI job to validate the code formatting.  After the
> .clang-format is merged, CI jobs will fail if the new code doesn't
> follow the standard.  The CI jenkins job will output the diff of the
> code that is not conforming to the console.
>
> We've tried our best to get the .clang-format file to match up with
> our requirements in
> https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md.
>
> The .clang-format file and the changes it did to the sdbusplus repo
> can be found in https://gerrit.openbmc-project.xyz/#/c/8461/.  Review
> comments appreciated.

Oh neat!

I gather the CI job is something like "parallel clang-format -i {} :::
`find . -name '*.c' -name '*.cpp' -name '*.h'`" and then check for any
changes?

-- 
Stewart Smith
OPAL Architect, IBM.

Re: openbmc coding standards validation in CI

[Andrew Geissler]

On Sun, Jan 14, 2018 at 5:06 PM, Stewart Smith
<stewart@linux.vnet.ibm.com> wrote:
> Andrew Geissler <geissonator@gmail.com> writes:
>> During the hackathon meetup, we decided to try and get something in
>> our CI jobs that would validate the code formatting and automatically
>> -1 the gerrit commit if it's not up to par.  The reasoning behind this
>> is we still have a lot of code review comments coming in this area,
>> which is wasteful for the reviewers.
>>
>> Ed, Adriana, and I all worked different pieces of it.  We're using the
>> clang-format tool to format (and then validate the format).  The idea
>> is you do the initial run of the tool against your repo and check the
>> .clang-format file in with your changes.  The presence of the file
>> will cause the CI job to validate the code formatting.  After the
>> .clang-format is merged, CI jobs will fail if the new code doesn't
>> follow the standard.  The CI jenkins job will output the diff of the
>> code that is not conforming to the console.
>>
>> We've tried our best to get the .clang-format file to match up with
>> our requirements in
>> https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md.
>>
>> The .clang-format file and the changes it did to the sdbusplus repo
>> can be found in https://gerrit.openbmc-project.xyz/#/c/8461/.  Review
>> comments appreciated.
>
> Oh neat!
>
> I gather the CI job is something like "parallel clang-format -i {} :::
> `find . -name '*.c' -name '*.cpp' -name '*.h'`" and then check for any
> changes?

Yep. you got it.
https://gerrit.openbmc-project.xyz/#/c/8455/4/scripts/format-code.sh
if interested in reviewing :)
The CI job will just call this script if it sees the .clang-format
file present in the repo.

>
> --
> Stewart Smith
> OPAL Architect, IBM.
>

Got some good comments on the format review, keep them coming!
(https://gerrit.openbmc-project.xyz/#/c/8461/)
This will most likely what forms a base in all our other repos so lets
get it right from the beginning.

Andrew