mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: ClangTidy for Mesos is here!
Date Wed, 08 Nov 2017 01:04:35 GMT
Thanks Michael and Benjamin!

On Sat, Nov 4, 2017 at 3:18 PM, Michael Park <mpark@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
>

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