Single-Source

Single-Source

[Patrick Venture]

Brad;

While working on updating the documentation, which holds a
clang-format example in it, within the markdown, I was curious if
there was a single-source of truth w.r.t configurations of this
nature?  I imagine it hasn't been an issue yet, but I'm going to
update clang-format files to use the header inclusion, once we settle
on the exact pattern, and it'd be useful to have a single-source file
where I could make the change, and copy that file out into places --
since I'm not sure of a reasonable automated approach.

Any suggestions or thoughts would be appreciated.

Patrick

Re: Single-Source

[William Kennington]

So I've taken the source from
https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md#clang-formatting
which closely matches what is in sdbusplus and modified it for the
projects I've created to better enforce the style guide. I don't think
it's possible to have a single source of truth at the moment since
some projects have more relaxed style. Although I don't think it would
be too big of a deal to force those projects to reformat. My current
delta between the original openbmc one and a more strict formatter is:
diff sdbusplus/.clang-format sdeventplus/.clang-format
20c20
< AlwaysBreakTemplateDeclarations: false
---
> AlwaysBreakTemplateDeclarations: true
45c45
< DerivePointerAlignment: true
---
> DerivePointerAlignment: false
68d67
< PointerAlignment: Right
70c69,70
< SortIncludes:    false
---
> SortIncludes:    true
> SortUsingDeclarations: true
On Wed, Aug 29, 2018 at 8:54 AM Patrick Venture <venture@google.com> wrote:
>
> Brad;
>
> While working on updating the documentation, which holds a
> clang-format example in it, within the markdown, I was curious if
> there was a single-source of truth w.r.t configurations of this
> nature?  I imagine it hasn't been an issue yet, but I'm going to
> update clang-format files to use the header inclusion, once we settle
> on the exact pattern, and it'd be useful to have a single-source file
> where I could make the change, and copy that file out into places --
> since I'm not sure of a reasonable automated approach.
>
> Any suggestions or thoughts would be appreciated.
>
> Patrick

Re: Single-Source

[Brad Bishop]

> On Aug 29, 2018, at 11:53 AM, Patrick Venture <venture@google.com> wrote:
> 
> Brad;
> 
> While working on updating the documentation, which holds a
> clang-format example in it, within the markdown, I was curious if
> there was a single-source of truth w.r.t configurations of this
> nature?  I imagine it hasn't been an issue yet, but I'm going to
> update clang-format files to use the header inclusion, once we settle
> on the exact pattern, and it'd be useful to have a single-source file
> where I could make the change, and copy that file out into places --
> since I'm not sure of a reasonable automated approach.
> 
> Any suggestions or thoughts would be appreciated.

Honestly I don’t think we should be telling maintainers how to style
the code they maintain.  Just that they have one and and provide a way
for contributors to adhere to it.

But I think I am in the minority with that viewpoint so I guess you
could put a clang-format in the docs repo?  Does that work?


> 
> Patrick

Re: Single-Source

[Patrick Venture]

On Wed, Aug 29, 2018 at 9:55 AM Brad Bishop <bradleyb@fuzziesquirrel.com> wrote:
>
>
>
> > On Aug 29, 2018, at 11:53 AM, Patrick Venture <venture@google.com> wrote:
> >
> > Brad;
> >
> > While working on updating the documentation, which holds a
> > clang-format example in it, within the markdown, I was curious if
> > there was a single-source of truth w.r.t configurations of this
> > nature?  I imagine it hasn't been an issue yet, but I'm going to
> > update clang-format files to use the header inclusion, once we settle
> > on the exact pattern, and it'd be useful to have a single-source file
> > where I could make the change, and copy that file out into places --
> > since I'm not sure of a reasonable automated approach.
> >
> > Any suggestions or thoughts would be appreciated.
>
> Honestly I don’t think we should be telling maintainers how to style
> the code they maintain.  Just that they have one and and provide a way
> for contributors to adhere to it.

I'm 50/50 on that one.  I think if you have a standard against which
all the code in a project follows, it's easier to work with -- the
sort of classic answer.  If we say, maintainers can do what they want
as long as contributors can adhere to it, then what's to say they
should follow the openbmc style?  I am sure most engineers have
differences with the openbmc style, so if they all end up with their
own variations of clang-format -- at least it's automated, but it's
still different to read every repo.  Of course, the clang-format
doesn't do variables names (right?), the style guide still dictates
that... I don't know, it seems like if everything under openbmc/ has
the same style it has increased coherency.

>
> But I think I am in the minority with that viewpoint so I guess you
> could put a clang-format in the docs repo?  Does that work?

That would work for me, so I can just copy that into my repo(s).

>
>
> >
> > Patrick

Re: Single-Source

[Patrick Venture]

On Wed, Aug 29, 2018 at 9:29 AM William Kennington <wak@google.com> wrote:
>
> So I've taken the source from
> https://github.com/openbmc/docs/blob/master/cpp-style-and-conventions.md#clang-formatting
> which closely matches what is in sdbusplus and modified it for the
> projects I've created to better enforce the style guide. I don't think
> it's possible to have a single source of truth at the moment since
> some projects have more relaxed style. Although I don't think it would
> be too big of a deal to force those projects to reformat. My current
> delta between the original openbmc one and a more strict formatter is:
> diff sdbusplus/.clang-format sdeventplus/.clang-format
> 20c20
> < AlwaysBreakTemplateDeclarations: false
> ---
> > AlwaysBreakTemplateDeclarations: true
> 45c45
> < DerivePointerAlignment: true
> ---
> > DerivePointerAlignment: false
> 68d67
> < PointerAlignment: Right
> 70c69,70
> < SortIncludes:    false
> ---
> > SortIncludes:    true
> > SortUsingDeclarations: true

Other than SortIncludes which is being handled at another level,
please update the model clang-format file in the documentation to make
it model more closely to the spec as you've described.

Obviously, a problem can be that the clang-format goal is regularly
changing, but there isn't an urgency in its usage.  I think eventually
consistent is a reasonable approach.  Some repositories will be behind
until some point, but it's not really a problem.

> On Wed, Aug 29, 2018 at 8:54 AM Patrick Venture <venture@google.com> wrote:
> >
> > Brad;
> >
> > While working on updating the documentation, which holds a
> > clang-format example in it, within the markdown, I was curious if
> > there was a single-source of truth w.r.t configurations of this
> > nature?  I imagine it hasn't been an issue yet, but I'm going to
> > update clang-format files to use the header inclusion, once we settle
> > on the exact pattern, and it'd be useful to have a single-source file
> > where I could make the change, and copy that file out into places --
> > since I'm not sure of a reasonable automated approach.
> >
> > Any suggestions or thoughts would be appreciated.
> >
> > Patrick

Re: Single-Source

[Ed Tanous]

> still different to read every repo.  Of course, the clang-format
> doesn't do variables names (right?)

clang-format can't do variable renames, but clang-tidy can.

I've been experimenting with it a bit, and it seems to work pretty well, 
but requires a lot more work than clang-format to get set up on a given 
repo.  Right now I'm using the CMAKE_EXPORT_COMPILE_COMMANDS flag to 
generate the metadata that it needs to run.  At some point, I want to 
look into doing that automatically in the build scripts, but I'm not 
there yet.

A .clang-tidy file that approximates the OpenBMC style guides naming 
standard is here:
https://github.com/openbmc/bmcweb/blob/master/.clang-tidy

Re: Single-Source

[Patrick Venture]

On Wed, Aug 29, 2018 at 11:26 AM Ed Tanous <ed.tanous@intel.com> wrote:
>
> > still different to read every repo.  Of course, the clang-format
> > doesn't do variables names (right?)
>
> clang-format can't do variable renames, but clang-tidy can.
>
> I've been experimenting with it a bit, and it seems to work pretty well,
> but requires a lot more work than clang-format to get set up on a given
> repo.  Right now I'm using the CMAKE_EXPORT_COMPILE_COMMANDS flag to
> generate the metadata that it needs to run.  At some point, I want to
> look into doing that automatically in the build scripts, but I'm not
> there yet.
>
> A .clang-tidy file that approximates the OpenBMC style guides naming
> standard is here:
> https://github.com/openbmc/bmcweb/blob/master/.clang-tidy

Cool.