mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <benjamin.mah...@gmail.com>
Subject Re: Project wide updates to #include statements
Date Sat, 27 Jun 2015 01:04:40 GMT
+1 to cody's suggestion for stout / libprocess.

Looking forward to seeing include style automated, will help us be more
effective with reviews. :)

It looks like we can further modify "include what you use" to capture stout
and libprocess headers, probably worth the effort:
https://github.com/google/styleguide/blob/gh-pages/cpplint/cpplint.py#L5604

On Thu, Jun 25, 2015 at 5:19 PM, Cody Maloney <cody@mesosphere.io> wrote:

> On Thu, Jun 25, 2015 at 5:01 PM Paul Brett <pbrett@twitter.com.invalid>
> wrote:
>
> > As Kapil has pointed out on MESOS-2927, all the library files should be
> > identified by '#include <...>' and all the project files should use
> > '#include "..."' - i don't know yet if we have been consistent about
> this.
> >
>
> They aren't consistent currently. For example:
> https://issues.apache.org/jira/browse/MESOS-1877
>
> I would actually suggest we switch to using <> everywhere. Reduces
> cognitive burden reading the code, and practically headers move over time,
> If something moves from src/mesos/master/master.hpp to
> include/mesos/master/master.hpp given the current <> vs "" rules, every
> file which includes that file needs to be updated.
>
> Cody
>
>
> > I have created the stout patch on this basis and plan to incorporate any
> > comments before moving onto libprocess.
> >
> > -- Paul
> >
> > On Thu, Jun 25, 2015 at 3:31 PM, Marco Massenzio <marco@mesosphere.io>
> > wrote:
> >
> > > As I mentioned in the review that Paul submitted, I've been working on
> > > cpplint.py to make it more Mesos-friendly.
> > > I have also submitted a few Pull Requests
> > > <https://github.com/google/styleguide/pulls> to the original github
> > repo,
> > > but got neither love nor attention.
> > >
> > > My fork is here: https://github.com/massenz/styleguide
> > > and the code in the `master` branch (
> > > https://github.com/massenz/styleguide/tree/master) has all my changes;
> > it
> > > currently works well with the existing code (in that, submitted, valid
> > > Mesos code does not raise errors) apart from the opening brace on a
> > newline
> > > for multi-line method declarations.
> > >
> > > Love to get contributions and pull requests folks, feel free to submit!
> > >
> > > An example CPPLINT.cfg that works with the code in `master` is
> something
> > > like this:
> > >
> > > $ cat CPPLINT.cfg
> > > # Apache Mesos cpplint custom file
> > >
> > > extensions=cpp,hpp
> > > access_keywords_indent=0
> > > headers=h,hpp
> > > custom_headers=mesos,process,stout
> > > set braces_newline
> > >
> > > PS - am I the only one to find it hilarious that code that supposedly
> > > checks on style correctness is written in some of the least readable,
> > badly
> > > PEP8-violating Python? :)
> > >
> > > *Marco Massenzio*
> > > *Distributed Systems Engineer*
> > >
> > > On Thu, Jun 25, 2015 at 3:18 PM, Paul Brett <pbrett@twitter.com.invalid
> >
> > > wrote:
> > >
> > > > ​The style guide prescribes the order of header file inclusions for
> the
> > > > project and requires that we #include or make explicit forward
> > > declarations
> > > > for any functions we use, however we were only  enforcing this at
> > review
> > > > time manually and not commit time.  I would like to turn on the
> checks
> > at
> > > > commit time, so I am in the process of raising changes against stout,
> > > > libprocess and mesos to bring the code base into compliance.  Once
> this
> > > is
> > > > completed, I propose to update cpplint.py and mesos-style.py to
> enforce
> > > the
> > > > style guide.
> > > >
> > > > Anyone interested can comment on the following tickets:
> > > >
> > > > https://issues.apache.org/jira/browse/MESOS-2926 Extend
> > > > mesos-style.py/cpplint.py to check #include files
> > > > https://issues.apache.org/jira/browse/MESOS-2927 Update mesos
> #include
> > > > headers
> > > > https://issues.apache.org/jira/browse/MESOS-2928 Update stout
> #include
> > > > headers
> > > > https://issues.apache.org/jira/browse/MESOS-2929 Update libprocess
> > > > #include
> > > > headers
> > > > ​
> > > >
> > > > -- Paul Brett
> > > >
> > >
> >
> >
> >
> > --
> > -- Paul Brett
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message