Thanks Michael and Benjamin! On Sat, Nov 4, 2017 at 3:18 PM, Michael Park <mp...@apache.org> wrote:
> We've had ClangTidy for Mesos, called MesosTidy for a while checked > into the codebase, but we hadn't actually enabled it on the CI. > > I've created a Mesos-Tidybot job (to accompany Mesos-Buildbot) on > our Apache CI, which will be reporting its status to the builds mailing > list. > > The following is an example output from MesosTidy in its last run: > > /tmp/SRC/3rdparty/libprocess/src/http.cpp:675:10: warning: redundant call > to 'data' [readability-redundant-string-cstr] > body = out.str().data(); > ^~~~~~~~~~~~~~~~~ > out.str() > > This warning is generated from a built-in ClangTidy check called > readability-redundant-string-cstr > <http://clang.llvm.org/extra/clang-tidy/checks/readability- > redundant-string-cstr.html>, > which checks for unnecessary > calls to `std::string::c_str` and `std::string::data`. > > Not a critical issue for us by any measure, but it's very important to > note that this is a __semantic__ check and I hope it gives you > a sense of the type of checks that ClangTidy is capable of. > > Another thing to note is that it even suggests a fix! > Namely, to change `out.str().data()` to `out.str()`. > I've put up a review to make this change here: r63560 > <https://reviews.apache.org/r/63560/> > > Today, we start with a very small number of checks: > > - `mesos-flags-inheritance` > - Ensures that Flags always inherits virtually for composability. > - `mesos-namespace-comments` > - Ensures that namespaces end with `// namespace foo {` > - `readability-redundant-string-cstr` > - As we saw above. > > We also have a `mesos-this-capture` check which will tell you when > you've captured the `this` pointer of a class in a libprocess continuation > but without a `defer` to `self`. This is disabled today however, since > there > are few false positives related to the uses of `process::loop`. I really > hope > to get around to updating the check and enable it soon! > > There are also a slew of checks that ship with ClangTidy that we'll be > enabling as well. http://clang.llvm.org/extra/clang-tidy/checks/list.html > Notably a lot of the `google-*` checks are relevant for us. > > You can also run `mesos-tidy` locally on your machine by running > `./support/mesos-tidy.sh`. > > Let's say this is an alpha launch. There are lots of things missing, > and things aren't going to be perfect, but I think that the framework > that is there today is a good start for us. We'll certainly be changing > things and improving as we go. > > Thank you to Benjamin Bannier for helping out with so much of this work. > He's been a really big help and an advocate for additional tools. > > Thanks, > > MPark >
