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

Ship it!



src/master/http.cpp (line 322)
<https://reviews.apache.org/r/36037/#comment143187>

    Another great candidate for the constants file: `Content-Type`



src/master/http.cpp (lines 349 - 350)
<https://reviews.apache.org/r/36037/#comment143188>

    this is a micro-nit (feel free to ignore)
    my impression is that the message would read better if you were to invert 
the order of the actual type and the name of the header:
    ```
    "Unsupported Content-Type: '" + contentType.get() +
    "'; Expecting one of (" + APPLICATION_PROTOBUF + 
    ", " + APPLICATION_JSON +")";
    ```
    
    so, if we add more, the comment can be easily extended.



src/master/validation.cpp (line 86)
<https://reviews.apache.org/r/36037/#comment143186>

    thanks for adding the comment!
    Ideally, this should be part of the javadoc at the top of the method?
    so people reading the API will see the @return value
    
    also, missing a `d`: *determined*



src/master/http_constants.hpp (line 26)
<https://reviews.apache.org/r/36037/#comment143189>

    note if you use the javadoc notation:
    ```
    /**
     * Supported Content-Tye and Accept headers
     */
    ```
    you will get this added to our Doxygen free of charge :)


This looks good, Isabel!

- Marco Massenzio


On July 1, 2015, 7:30 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36037/
> -----------------------------------------------------------
> 
> (Updated July 1, 2015, 7:30 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Hindman, Ben Mahler, Marco 
> Massenzio, and Vinod Kone.
> 
> 
> Bugs: MESOS-2860
>     https://issues.apache.org/jira/browse/MESOS-2860
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> -------
> 
> Adding a call route with HTTP request header validations
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a064d17 
>   src/master/http.cpp 2be613b 
>   src/master/http_constants.hpp PRE-CREATION 
>   src/master/http_constants.cpp PRE-CREATION 
>   src/master/master.hpp af83d3e 
>   src/master/master.cpp 34ce744 
>   src/master/validation.hpp 469d6f5 
>   src/master/validation.cpp 9d128aa 
>   src/tests/call_tests.cpp PRE-CREATION 
>   src/tests/mesos.hpp 9157ac0 
> 
> Diff: https://reviews.apache.org/r/36037/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Reply via email to