You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2012/11/02 18:26:40 UTC

Re: Review Request: Created a CHECK_SOME macro.

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



third_party/libprocess/include/process/http.hpp
<https://reviews.apache.org/r/7772/#comment28083>

    CHECK(1 <= UCHAR_MAX) << ...blah... 
    
    is more clear?



third_party/libprocess/include/stout/numify.hpp
<https://reviews.apache.org/r/7772/#comment28084>

    ditto?


- Vinod Kone


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Ben Mahler <be...@gmail.com>.

> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/numify.hpp, lines 25-28
> > <https://reviews.apache.org/r/7772/diff/1/?file=182211#file182211line25>
> >
> >     ditto?

I think benh wants the stout library to be standalone, in that it shouldn't require glog, where CHECK is defined.
This is a perfect use case for the ABORT function I created in another review.


> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, lines 307-312
> > <https://reviews.apache.org/r/7772/diff/1/?file=182210#file182210line307>
> >
> >     CHECK(1 <= UCHAR_MAX) << ...blah... 
> >     
> >     is more clear?

Do we want to use glog in our libprocess/include files?
If so, I'll switch it, but I'm not seeing many instances of CHECK in these includes, so I hesitated.


- Ben


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


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>


Re: Review Request: Created a CHECK_SOME macro.

Posted by Vinod Kone <vi...@gmail.com>.

> On Nov. 2, 2012, 5:26 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/process/http.hpp, lines 307-312
> > <https://reviews.apache.org/r/7772/diff/1/?file=182210#file182210line307>
> >
> >     CHECK(1 <= UCHAR_MAX) << ...blah... 
> >     
> >     is more clear?
> 
> Ben Mahler wrote:
>     Do we want to use glog in our libprocess/include files?
>     If so, I'll switch it, but I'm not seeing many instances of CHECK in these includes, so I hesitated.

aha. i see. sgtm then


- Vinod


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


On Oct. 30, 2012, 1:23 a.m., Ben Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7772/
> -----------------------------------------------------------
> 
> (Updated Oct. 30, 2012, 1:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Description
> -------
> 
> This adds consistency with what we've done with ASSERT_SOME, EXPECT_SOME.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_Log.cpp e9ffccf8912977ff89cc5cb1ea1369ea5ce25647 
>   src/log/coordinator.cpp 31d48e901825b44de0b5e66394cd9020108b45b6 
>   src/log/log.hpp db026dc1edcf0bfe059dcff4036a1278d9933c26 
>   src/log/log.cpp fcc343ef0e7cab3152d49dd837ce4da0347c8cd8 
>   src/log/main.cpp 69d9443702e2ddf3b868957fe8f2712346a677de 
>   src/log/replica.cpp b39623e8a9cb44cf4889b4d3351e952227810e54 
>   src/logging/logging.hpp 8e8a4f6f92644c4db2d0d59925a930b6f0f72961 
>   src/master/main.cpp 74c955f262179d52408d6b0eaada4d7801bcc193 
>   src/slave/main.cpp 70bb71e427aaa5e1ebb1ae132c9aad3ebe9be34c 
>   src/slave/paths.hpp 98e7fd402919c50a26f69a2f1a1904cb877c5f43 
>   src/slave/process_based_isolation_module.cpp 744832625d6d1898eb2b31b184f5d0c68408835d 
>   src/slave/slave.cpp 0321bc516166aacfd261c48f1f4293622d18ae0e 
>   src/slave/state.cpp 65d91b889b84152ed12c4f2dd623b537242258df 
>   src/tests/flags.hpp add9e0a1ec8c73e6556f9da2019e409edda305b3 
>   src/tests/script.cpp 47fe9d72fd73a3cf73e59b7a0ba4818690e1f130 
>   src/tests/slave_state_tests.cpp 7d09507d37f41c77f32ec04c7ecfc9657f196a30 
>   src/tests/utils.cpp deef0bba1374cf8a8591813877a6170974d6de94 
>   src/zookeeper/group.cpp 876b2ce4e94bfe45902ef86c3143b68ff30029f0 
>   third_party/libprocess/include/process/future.hpp 49767c57c45f2a4d5c293b1ce0dcef3e40ab2e9f 
>   third_party/libprocess/include/process/http.hpp f8d0a5886c77eaf885732346b89be326e89afad6 
>   third_party/libprocess/include/stout/numify.hpp 9f49e80c4f4d366264a699b12f84fc114f2dfe6a 
> 
> Diff: https://reviews.apache.org/r/7772/diff/
> 
> 
> Testing
> -------
> 
> CentOS: make check
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>