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


This looks pretty good - there are just some comment nits I would like to get 
fixed.


3rdparty/libprocess/include/process/firewall.hpp (line 31)
<https://reviews.apache.org/r/35353/#comment140602>

    s/in/is/ ... well actually, lets really use my suggestion unless you got 
some good reason to not like it :)
    
    ```
    /**
     * A 'FirewallRule' describes an interface which provides control
     * over incoming HTTP requests while also taking the underlying
     * connection into account.
     *
     * Concrete classes based on this interface must implement the
     * 'apply' method.
     *
     * Rules can be installed using the free function
     * 'process::firewall::install()' defined in 'process.hpp'.
     */
    ```



3rdparty/libprocess/include/process/firewall.hpp (lines 47 - 58)
<https://reviews.apache.org/r/35353/#comment140751>

    Lets boil this one down a little;
    
    ```
      /**
       * Verify rule by applying it to an HTTP request and its underlying
       * socket connection.
       *
       * @param socket Socket used to deliver the HTTP request.
       * @param request HTTP request made by the client to libprocess.
       * @return If the rule verification fails, i.e. the rule didn't
       *     match, the returned error is set with an explanation for the
       *     failure. Otherwise None is returned.
       */
    ```



3rdparty/libprocess/include/process/firewall.hpp (line 67)
<https://reviews.apache.org/r/35353/#comment140752>

    I think of this class as something that disables any endpoints referenced 
by paths -- as such, we do not expect *disabled* endpoint paths as parameters. 
    
    s/disabled//


- Till Toenshoff


On June 15, 2015, 3:51 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35353/
> -----------------------------------------------------------
> 
> (Updated June 15, 2015, 3:51 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/firewall.hpp 
> 16ed852d07344e53c6f9d30d4cd7d99e07c6606d 
>   3rdparty/libprocess/include/process/process.hpp 
> 6a0b21d67912a40e0ec3220fdb250930be1979b2 
>   3rdparty/libprocess/src/process.cpp 
> f919b997287435381dbe34cb5bfdf73641ebeb23 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 660af45e7fd45bdf5d43ad9aa54477fd94f87058 
> 
> Diff: https://reviews.apache.org/r/35353/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to