You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/07/10 03:30:54 UTC

Review Request 67866: Apply the `override` keyword to stout.

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

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 stout.


Diffs
-----

  3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
  3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 


Diff: https://reviews.apache.org/r/67866/diff/1/


Testing
-------

make check (Fedora 28)


Thanks,

James Peach


Re: Review Request 67866: Apply the `override` keyword to stout.

Posted by James Peach <jp...@apache.org>.

> On July 10, 2018, 8:02 a.m., Benjamin Bannier wrote:
> > Running clang-tidy on this produced more changes for me
> > 
> >     3rdparty/stout/tests/main.cpp                      |  2 +-
> >     3rdparty/stout/tests/os/rmdir_tests.cpp            |  2 +-
> >     3rdparty/stout/tests/os/sendfile_tests.cpp         |  2 +-
> >     3rdparty/stout/tests/subcommand_tests.cpp          |  4 +--
> >     
> > 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?

I tried a bunch of different settings, but still no guarantee I got all the cases :)


- James


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


On July 12, 2018, 5:02 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> -----------------------------------------------------------
> 
> (Updated July 12, 2018, 5:02 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 stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 
>   3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/sendfile_tests.cpp cc1fa113eae5c2c68e2f4f21c35b700650db787c 
>   3rdparty/stout/tests/subcommand_tests.cpp 211d28df1bba069c848000a19d3f6038e71d8159 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 67866: Apply the `override` keyword to stout.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67866/#review205898
-----------------------------------------------------------



Running clang-tidy on this produced more changes for me

    3rdparty/stout/tests/main.cpp                      |  2 +-
    3rdparty/stout/tests/os/rmdir_tests.cpp            |  2 +-
    3rdparty/stout/tests/os/sendfile_tests.cpp         |  2 +-
    3rdparty/stout/tests/subcommand_tests.cpp          |  4 +--
    
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?

- Benjamin Bannier


On July 10, 2018, 5:30 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> -----------------------------------------------------------
> 
> (Updated July 10, 2018, 5:30 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 stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 67866: Apply the `override` keyword to stout.

Posted by Benjamin Bannier <be...@mesosphere.io>.

> On July 16, 2018, 5:37 p.m., Benjamin Bannier wrote:
> > Ship It!

> 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?


- Benjamin


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


On July 12, 2018, 7:02 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> -----------------------------------------------------------
> 
> (Updated July 12, 2018, 7:02 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 stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 
>   3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/sendfile_tests.cpp cc1fa113eae5c2c68e2f4f21c35b700650db787c 
>   3rdparty/stout/tests/subcommand_tests.cpp 211d28df1bba069c848000a19d3f6038e71d8159 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 67866: Apply the `override` keyword to stout.

Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67866/#review206107
-----------------------------------------------------------


Ship it!




Ship It!

- Benjamin Bannier


On July 12, 2018, 7:02 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> -----------------------------------------------------------
> 
> (Updated July 12, 2018, 7:02 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 stout.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 
>   3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/sendfile_tests.cpp cc1fa113eae5c2c68e2f4f21c35b700650db787c 
>   3rdparty/stout/tests/subcommand_tests.cpp 211d28df1bba069c848000a19d3f6038e71d8159 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/2/
> 
> 
> Testing
> -------
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Re: Review Request 67866: Apply the `override` keyword to stout.

Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67866/
-----------------------------------------------------------

(Updated July 12, 2018, 5:02 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 stout.


Diffs (updated)
-----

  3rdparty/stout/include/stout/lambda.hpp 252de895ac679715ef20015d8887b1a78f264fe8 
  3rdparty/stout/include/stout/tests/utils.hpp e7336a586cc3ad4147cfe963604059026fc1e3f1 
  3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
  3rdparty/stout/tests/os/rmdir_tests.cpp 518afc0ddedc4c53d01ac08a36234964f088420d 
  3rdparty/stout/tests/os/sendfile_tests.cpp cc1fa113eae5c2c68e2f4f21c35b700650db787c 
  3rdparty/stout/tests/subcommand_tests.cpp 211d28df1bba069c848000a19d3f6038e71d8159 


Diff: https://reviews.apache.org/r/67866/diff/2/

Changes: https://reviews.apache.org/r/67866/diff/1-2/


Testing
-------

make check (Fedora 28)


Thanks,

James Peach