You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2015/12/01 00:27:36 UTC

Re: Review Request 40497: Add hex number support to numify()

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp (lines 42 - 47)
<https://reviews.apache.org/r/40497/#comment167875>

    I would write the logics here in the following way:
    
    ```
    if (!ss.fail() && ss.eof()) {
      return result;
    }
    
    return Error("...");
    ```



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 25)
<https://reviews.apache.org/r/40497/#comment167876>

    We typically use 10u instead of 10U.



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 41)
<https://reviews.apache.org/r/40497/#comment167879>

    Please use EXPECT_ERROR here. Ditto for the followings.



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 42)
<https://reviews.apache.org/r/40497/#comment167877>

    Kill extra lines here.



3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp (line 45)
<https://reviews.apache.org/r/40497/#comment167878>

    Can you also add a floating point test here:
    
    ```
    EXPECT_ERROR(numify<double>("0x10.9"));
    ```


- Jie Yu


On Nov. 30, 2015, 9:57 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40497/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 9:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Ian Downes, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> numify() in libprocess uses boost lexical_cast() to cast numbers, but it can not cast a hex number. This patch adds hex support to numify().
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 5a0ffc38d4194b9f9c53dc1cf0c90ca7bbae2afd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/numify.hpp 440181c25d9d132f48c7a4888dc1dbb0157dc6c8 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 14fb644b38a5cbb8cde74aab39e84305f6ab7041 
>   3rdparty/libprocess/3rdparty/stout/tests/numify_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40497/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>