From reviews-return-8507-apmail-mesos-reviews-archive=mesos.apache.org@mesos.apache.org Mon Aug 31 14:35:03 2015 Return-Path: X-Original-To: apmail-mesos-reviews-archive@minotaur.apache.org Delivered-To: apmail-mesos-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 9DA0E174F5 for ; Mon, 31 Aug 2015 14:35:03 +0000 (UTC) Received: (qmail 62472 invoked by uid 500); 31 Aug 2015 14:35:03 -0000 Delivered-To: apmail-mesos-reviews-archive@mesos.apache.org Received: (qmail 62451 invoked by uid 500); 31 Aug 2015 14:35:03 -0000 Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@mesos.apache.org Delivered-To: mailing list reviews@mesos.apache.org Received: (qmail 62434 invoked by uid 99); 31 Aug 2015 14:35:03 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 31 Aug 2015 14:35:03 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id B975F26BACE; Mon, 31 Aug 2015 14:35:01 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============3880923062356580282==" MIME-Version: 1.0 Subject: Re: Review Request 37871: SSL tests refactoring From: "Joris Van Remoortere" To: "Joris Van Remoortere" , "Timothy Chen" Cc: "Jojy Varghese" , "mesos" Date: Mon, 31 Aug 2015 14:35:01 -0000 Message-ID: <20150831143501.16295.172@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Joris Van Remoortere" X-ReviewGroup: mesos X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/37871/ X-Sender: "Joris Van Remoortere" References: <20150831143202.16295.22065@reviews.apache.org> In-Reply-To: <20150831143202.16295.22065@reviews.apache.org> Reply-To: "Joris Van Remoortere" X-ReviewRequest-Repository: mesos --===============3880923062356580282== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- 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) Let's leave `openssl.hpp` in here for now. 3rdparty/libprocess/include/Makefile.am (lines 66 - 67) 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) Move this down into its own block. 3rdparty/libprocess/include/process/ssl/gtest.hpp (line 35) 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) Move this file to `src/ssl/utilities.cpp` to match the include declaration file. 3rdparty/libprocess/src/openssl_util.cpp (line 23) 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) Move this subdirectory to a separate block. Also include `` 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 > > --===============3880923062356580282==--