----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52415/#review151025 -----------------------------------------------------------
Fix it, then Ship it! src/slave/containerizer/mesos/linux_launcher.cpp (lines 584 - 587) <https://reviews.apache.org/r/52415/#comment219151> If you agree with the comments below, you can leave off the `JOIN` here. src/slave/containerizer/mesos/paths.hpp (line 41) <https://reviews.apache.org/r/52415/#comment219150> Should we use an `enum class` here? Without it, the enum will not be typed properly under `Mode`, so you could write things like: `containerizer::paths::JOIN` (which you currently do, and is totally legal) With `enum class` you are forced to write: `containerizer::paths::Mode::JOIN` Not sure which is better, but it would be good to be explicit about it. src/slave/containerizer/mesos/paths.hpp (lines 57 - 60) <https://reviews.apache.org/r/52415/#comment219149> To keep similar semantics as before, maybe it makes sense to set defaults for separator and mode: ``` std::string buildPath( const ContainerID& containerId, const std::string& separator = "", const Mode& mode = Mode::JOIN); ``` - Kevin Klues On Sept. 30, 2016, 5:14 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52415/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2016, 5:14 a.m.) > > > Review request for mesos, Benjamin Hindman and Kevin Klues. > > > Repository: mesos > > > Description > ------- > > Extended buildPath to support more modes. > > > Diffs > ----- > > src/Makefile.am f093000e0282a8d5ac17e7ba33711690ccdfe68a > src/slave/containerizer/mesos/linux_launcher.cpp > 1bce077e4aa97425b9cbdf8576a5dd52851c044e > src/slave/containerizer/mesos/paths.hpp > 1051c219c55253d03199045b6d2f43377ae93e53 > src/slave/containerizer/mesos/paths.cpp > 6c6b4dcc39fbc00485552caab88457918e622e08 > src/tests/containerizer/mesos_containerizer_paths_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/52415/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
