+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 <[email protected]> wrote: > On Thu, Jun 25, 2015 at 5:01 PM Paul Brett <[email protected]> > 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 <[email protected]> > > 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 <[email protected] > > > > > 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 > > >
