mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joris Van Remoortere" <joris.van.remoort...@gmail.com>
Subject Re: Review Request 37871: SSL tests refactoring
Date Mon, 31 Aug 2015 14:35:01 GMT

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

Ship it!


Thanks for taking this on Jojy!
I will fix these for you before I commit. I just wanted to note them so you know what changed.


3rdparty/libprocess/Makefile.am (line 85)
<https://reviews.apache.org/r/37871/#comment152815>

    Let's leave `openssl.hpp` in here for now.



3rdparty/libprocess/include/Makefile.am (lines 66 - 67)
<https://reviews.apache.org/r/37871/#comment152814>

    Makefiles tend to get messy in developers' editors because they use tabs. Try to open
in vim to make sure the backslashes get alligned correctly.
    
    I know we tend to call subdirectories out in blocks. In the makefile that doesn't seem
to be the case. Either move the other subdirs into separate blocks as well, or just push these
into their alphabetical order.



3rdparty/libprocess/include/process/ssl/gtest.hpp (line 29)
<https://reviews.apache.org/r/37871/#comment152813>

    Move this down into its own block.



3rdparty/libprocess/include/process/ssl/gtest.hpp (line 35)
<https://reviews.apache.org/r/37871/#comment152812>

    let's move this above the includes since we can't assume the openssl/* headers are included.



3rdparty/libprocess/src/openssl_util.cpp (lines 1 - 14)
<https://reviews.apache.org/r/37871/#comment152808>

    Move this file to `src/ssl/utilities.cpp` to match the include declaration file.



3rdparty/libprocess/src/openssl_util.cpp (line 23)
<https://reviews.apache.org/r/37871/#comment152806>

    Move this to the top of the file to meet the "first include the file you are implementing"
pattern.



3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 22 - 23)
<https://reviews.apache.org/r/37871/#comment152805>

    Move this subdirectory to a separate block.
    Also include `<process/ssl/utilities.hpp>`


Let's add a conditional compilation guard to `utilities.hpp` as well.

- Joris Van Remoortere


On Aug. 31, 2015, 2:32 p.m., Jojy Varghese wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37871/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2015, 2:32 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Timothy Chen.
> 
> 
> Bugs: MESOS-3337
>     https://issues.apache.org/jira/browse/MESOS-3337
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Refactored libprocess SSL tests to be available for reuse by other projects.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/Makefile.am 7ef515848508c2e84ab7607595f635f67e24b19b 
>   3rdparty/libprocess/include/Makefile.am 3b6108dd37d23bd5104162e9e8f4a3aa0518cdcd 
>   3rdparty/libprocess/include/process/ssl/gtest.hpp PRE-CREATION 
>   3rdparty/libprocess/src/openssl_util.hpp  
>   3rdparty/libprocess/src/openssl_util.cpp 42b2860beccff929563ed05905da226b781918b7 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 7a316bc10575325ffe732fcc87d72d15a4fc5eaf

> 
> Diff: https://reviews.apache.org/r/37871/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>


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