mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 67867: Apply the `override` keyword to libprocess.
Date Tue, 10 Jul 2018 08:02:41 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67867/#review205899
-----------------------------------------------------------



Running clang-tidy on this produced more changes for me

    3rdparty/libprocess/examples/example.cpp           |  4 +--
    3rdparty/libprocess/src/http.cpp                   |  6 ++--
    3rdparty/libprocess/src/process.cpp                |  6 ++--
    3rdparty/libprocess/src/tests/benchmarks.cpp       | 10 +++----
    3rdparty/libprocess/src/tests/http_tests.cpp       |  8 ++---
    3rdparty/libprocess/src/tests/metrics_tests.cpp    |  2 +-
    3rdparty/libprocess/src/tests/process_tests.cpp    | 16 +++++-----
    3rdparty/libprocess/src/tests/profiler_tests.cpp   |  2 +-
    3rdparty/libprocess/src/tests/socket_tests.cpp     |  2 +-

When doing automated clang-tidy refactors it is also always very useful to call out how the
compilation database was generated (e.g., what flags were used). Could you add that to either
the commit message or the Testing done section?

When looking instances methods declared `virtual` I noticed that clang-tidy didn't change
e.g.,

    3rdparty/libprocess/include/process/grpc.hpp:    virtual ~RuntimeProcess();
    
Could you audit what it missed elsewhere as well? We should also update examples, e.g., in
`README.md`. We should make these additional changes in another patch to distinguish automated
and manual fixes.

- Benjamin Bannier


On July 10, 2018, 5:31 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67867/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:31 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, Mesos Reviewbot,
Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
>     https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Apply the `override` keyword to libprocess.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/async.hpp 116c90c01655cd1df67616d9c927f6769f5c92da

>   3rdparty/libprocess/include/process/collect.hpp 5263f0134808a0161b16b3b45871f711b0f5cb45

>   3rdparty/libprocess/include/process/event.hpp ec64eb786b6cf30bf03ba07a30c6e44d2db9173a

>   3rdparty/libprocess/include/process/filter.hpp 3f4a8272c451e333ebf9ff79c5f0fee329a8450a

>   3rdparty/libprocess/include/process/firewall.hpp 0a7b985380893820d0193f7b2eb657ee8c07430e

>   3rdparty/libprocess/include/process/gmock.hpp b5ead1c2ecacdb468a319c98edd2ffc831a9f74c

>   3rdparty/libprocess/include/process/gtest.hpp 79ea2b1f6a2a16ff9c5aae281c900e50940877bf

>   3rdparty/libprocess/include/process/help.hpp 3393b71c484bb8c55c6a4cd2a27dcb798e841a80

>   3rdparty/libprocess/include/process/limiter.hpp b4b6a7bb48aa19fa40c0ea9f8759b8f553f43ca6

>   3rdparty/libprocess/include/process/logging.hpp aad7ce8caa193e6aff1a295fef899b49fcfce7b0

>   3rdparty/libprocess/include/process/metrics/counter.hpp 15aeeb5710636d4e11b862faee50fd6ea4d1cb07

>   3rdparty/libprocess/include/process/metrics/metrics.hpp f9b72029b2c85826c91b1d7656b0af94dc87010c

>   3rdparty/libprocess/include/process/metrics/pull_gauge.hpp 5c2a227425f31c439bd48b58171d4038a7f04e5f

>   3rdparty/libprocess/include/process/metrics/timer.hpp 0a9c0227c457c6c81a59f65f901a5464ee00983d

>   3rdparty/libprocess/include/process/process.hpp c36df991b6a2c120ab0562e8ff907f9fbf8630d1

>   3rdparty/libprocess/include/process/protobuf.hpp 5a75a83d294e2d14f81503b113724893c0e140f3

>   3rdparty/libprocess/include/process/reap.hpp 628bc0a6ec59bfe6bfa7c54906ea266ac31a71a7

>   3rdparty/libprocess/include/process/sequence.hpp 24712b16953347eb05a28ad50fbdfad17bf64f19

> 
> 
> Diff: https://reviews.apache.org/r/67867/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>


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