> On April 21, 2015, 2:51 a.m., Marco Massenzio wrote:
> > src/master/http.cpp, line 298
> > <https://reviews.apache.org/r/30612/diff/12/?file=935462#file935462line298>
> >
> >     argubaly this should be DEBUG (but don't really feel strongly about 
> > this)

Agree with you, but every other request uses `LOG(INFO)` and mesos as a general 
rule goes for consitency.


> On April 21, 2015, 2:51 a.m., Marco Massenzio wrote:
> > src/master/http.cpp, line 300
> > <https://reviews.apache.org/r/30612/diff/12/?file=935462#file935462line300>
> >
> >     can we have all routes patters as CONSTANTS defined in the class 
> > header? definitely easier to debug etc.
> 
> Alexander Rukletsov wrote:
>     Marco, we tend not to use constants for non-POD types: 
> https://issues.apache.org/jira/browse/MESOS-1023
>     An example of a prefered way is `MAX_REAP_INTERVAL()` from `reap.hpp`. 
>     
>     In this particular case, why do you want to increase the visibility scope 
> of this constant?

I don't think this pattern should be move out of the function. There are 
multiple reasons for this:

1. The variable `pattern` is not used outside the scope of this method, so 
there's no need to take it outside.
2. `pattern` doesn't represent the path of the handler, since it is redirected 
from `/master/frameworks`.
3. We avoid having static class objects (though it could be converted to a 
`const char*`).
4. How is it easier to debug? I would probably have to go to another file if I 
want to check the contents of `pattern` as opposed from looking in the local 
function itself.
5. You can check the first niq's review on 
[29883](https://reviews.apache.org/r/29883/) as to why it is not capitalized.


- Alexander


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


On April 20, 2015, 6:27 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30612/
> -----------------------------------------------------------
> 
> (Updated April 20, 2015, 6:27 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Marco Massenzio, Niklas Nielsen, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-2157
>     https://issues.apache.org/jira/browse/MESOS-2157
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds endpoints for paths /master/frameworks/{framework}/tasks/{task}.
> 
> Adds tests for the endpoints.
> 
> Works with [29883](https://reviews.apache.org/r/29883).
> 
> Builds on the abandoned patch 14286.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 00c22c43bd1f6cef7963b2ffa9c095c6cbd01cd3 
>   src/master/master.hpp c10e7c08c191acef9d31d98018a47a2a952a4dfc 
>   src/master/master.cpp e30b951eda2b3b0d5b2a80716f0b32c6bbe041bc 
>   src/tests/master_tests.cpp 32b1e9bb58d8046e5363fafe2ab8f662b6c9a666 
> 
> Diff: https://reviews.apache.org/r/30612/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Reply via email to