> On Nov. 16, 2017, 11:08 p.m., Michael Park wrote: > > src/tests/fetcher_tests.cpp > > Line 469 (original), 473 (patched) > > <https://reviews.apache.org/r/60628/diff/6/?file=1864359#file1864359line473> > > > > These would ideally be a `url::join(...)`, right? > > I think for now we just use `strings::join('/', ...)` > > in the codebase. Could we do that here and below, for now rather than > > introducing a `url::join`? > > Jeff Coffler wrote: > I guess, if we had a url::join(), but we don't, and I'm not really sure > we need it (URLs always have `/` characters). I'm not 100% sure why you're > suggesting strings::join, as that would just be more run-time work. The code > is already building a string, and we're adding "/test" to it. Why do the work > to build an extra substring ("/" + "test") and then add that, when we can > just add the two strings together directly? > > Note that path::join was never correct, as we never wanted > http://blah\test! > > Or are you suggesting `strings::join(http.process->self().id, "/test")` > rather than `http.process->self().id + "/test")`? > > Jeff Coffler wrote: > If you are suggesting `strings::join(http.process->self().id, "/test")`, > is there a reason? Part of the coding standard I'm not aware of? Or you just > prefer the syntax? > > Andrew Schwartzmeyer wrote: > I think he's just suggesting: > > ``` > strings::join("/", http.process->self().id, "test"); > ``` > > Like > [this](https://github.com/apache/mesos/blob/02758c4e75483a0cd135fa465d1704d793bd4e48/3rdparty/stout/tests/strings_tests.cpp#L464). > > Like "join these strings with this separator".
Oh, I get it. Okay, no problem. Retested on both Windows and Linux, tests passed for Fetcher. Will wait for all tests to complete, then will push latest sources up for merge to master. - Jeff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60628/#review191264 ----------------------------------------------------------- On Nov. 6, 2017, 6:09 p.m., Jeff Coffler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60628/ > ----------------------------------------------------------- > > (Updated Nov. 6, 2017, 6:09 p.m.) > > > Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, Li > Li, and Michael Park. > > > Bugs: MESOS-6705 > https://issues.apache.org/jira/browse/MESOS-6705 > > > Repository: mesos > > > Description > ------- > > Tests for tar, gzip, and such won't be working on Windows for > the time being. Thoughts are to provide this capability to the > Fetcher in a cross-platform manner via a programmatic code library > rather than Linux-specific command line tools (tar, gzip, etc). > See MESOS-8064 for more details. > > In the short term, zip and unzip will work since PowerShell can > support that natively. > > > Diffs > ----- > > src/launcher/CMakeLists.txt c7a83d476efe13d65fa529e7676b6488eb48f44b > src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d > src/slave/containerizer/fetcher.cpp > ba5b0979aa427c5b3dbacf39661d6027da13e0d6 > src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76 > src/tests/fetcher_tests.cpp df9d2d9586a6457004506c4e2a972ccfc912c7c5 > > > Diff: https://reviews.apache.org/r/60628/diff/6/ > > > Testing > ------- > > See upstream. > > > Thanks, > > Jeff Coffler > >
