Re: Review Request 30531: Remove strings::format and unnecessary constants from paths

[Jie Yu]

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


Can we seperate the tests changes in this patch? It is a good practice not to 
change tests for refactors (if not necessary) so that we can capture potentiall 
regressions and have more confidence about the refactor?

The rest looks good to me! Thanks for the cleanup!


src/slave/paths.cpp
<https://reviews.apache.org/r/30531/#comment116331>

    const string& directory



src/slave/paths.cpp
<https://reviews.apache.org/r/30531/#comment116332>

    const string& last



src/slave/paths.cpp
<https://reviews.apache.org/r/30531/#comment116333>

    ditto.



src/slave/paths.cpp
<https://reviews.apache.org/r/30531/#comment116334>

    ditto.



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116339>

    Two lines between top level functions. Here and everywhere else.



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116335>

    const string&



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116337>

    `{` should be in next line. Here and everywhere else.



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116340>

    const string&



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116346>

    const string&



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116342>

    The format here is a little jagged, how about
    
    ```
    EXPECT_EQ(
        path::join(
            executorsRoot,
            executorId.value(),
            "runs",
            containerId.value()),
        paths::getExecutorRunPath(
            rootDir,
            slaveId,
            frameworkId,
            executorId,
            containerId));
    ```



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116344>

    Ditto here on the jaggedness.



src/tests/paths_tests.cpp
<https://reviews.apache.org/r/30531/#comment116345>

    const string&


- Jie Yu


On Feb. 2, 2015, 11:40 p.m., Dominic Hamon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30531/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2015, 11:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2314
>     https://issues.apache.org/jira/browse/MESOS-2314
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Remove strings::format and unnecessary constants from paths
> 
> 
> Diffs
> -----
> 
>   src/slave/paths.hpp 4d6897f9eebcd6852f0223bae23a29729a42e750 
>   src/slave/paths.cpp fe951abac1254748dbd5bdd81b7e50da75afad6d 
>   src/tests/paths_tests.cpp 417aa51b99d54055ad7254b4f274fb59d0794ca6 
> 
> Diff: https://reviews.apache.org/r/30531/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dominic Hamon
> 
>