Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review55162 --- Ship it! Looks great. I can add the blank line and commit it. 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp https://reviews.apache.org/r/25818/#comment95547 Nit: Put a blank line before this comment too. - Adam B On Sept. 29, 2014, 11:01 a.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 29, 2014, 11:01 a.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 29, 2014, 6:01 p.m.) Review request for mesos, Adam B and Dominic Hamon. Changes --- Address Adam's comments Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review54852 --- Patch looks great! Reviews applied: [25818] All tests passed. - Mesos ReviewBot On Sept. 29, 2014, 6:01 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 29, 2014, 6:01 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
On Sept. 29, 2014, 5:21 a.m., Adam B wrote: 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp, line 34 https://reviews.apache.org/r/25818/diff/5/?file=706722#file706722line34 Excuse my ignorance of variadic templates, but could one write path::join(/) or path::join()? If those would compile, they would be easy edge cases. path::join(/); doesn't compile (Although I could make it if we want). It would require another specialization of strings::join() which takes only one argument (The string to join by), or an explicit specialization of path::join() to deal with the single element case (Take a string and just return it). On Sept. 29, 2014, 5:21 a.m., Adam B wrote: 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp, lines 27-29 https://reviews.apache.org/r/25818/diff/5/?file=706722#file706722line27 Aren't two of these lines the same? Am I missing something? The two identical lines are indeed. Removed the second one. - Cody --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review54793 --- On Sept. 29, 2014, 6:01 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 29, 2014, 6:01 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review54789 --- Patch looks great! Reviews applied: [25818] All tests passed. - Mesos ReviewBot On Sept. 27, 2014, 10:53 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 27, 2014, 10:53 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review54793 --- Ship it! LGTM. Just minor comments. 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp https://reviews.apache.org/r/25818/#comment95061 Nit: s/don't/Don't/ and add a blank line before comments. 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp https://reviews.apache.org/r/25818/#comment95062 Aren't two of these lines the same? Am I missing something? 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp https://reviews.apache.org/r/25818/#comment95063 Excuse my ignorance of variadic templates, but could one write path::join(/) or path::join()? If those would compile, they would be easy edge cases. - Adam B On Sept. 27, 2014, 3:53 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 27, 2014, 3:53 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 27, 2014, 10:53 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 26, 2014, 11:22 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description (updated) --- Switch path::join() to be variadic. Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 26, 2014, 11:22 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description (updated) --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review54762 --- Bad patch! Reviews applied: [25789] Failed command: git apply --index 25789.patch Error: error: patch failed: 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp:15 error: 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp: patch does not apply - Mesos ReviewBot On Sept. 26, 2014, 11:22 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 26, 2014, 11:22 p.m.) Review request for mesos, Adam B and Dominic Hamon. Repository: mesos-git Description --- Switch path::join() to be variadic. Updated now that 25789 has landed Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8d 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0b 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 14567f1 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review53943 --- Bad patch! Reviews applied: [25789, 25818] Failed command: ./support/mesos-style.py Error: Checking 507 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp:29: Lines should be = 80 characters long [whitespace/line_length] [2] Total errors found: 1 - Mesos ReviewBot On Sept. 19, 2014, 5:32 a.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 19, 2014, 5:32 a.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Switch path::join() to be variadic Also fix a bug in strings::trim() where when the string was just characters to be trimmed, nothing would be trimmed. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a24d920c809e36a2a3da1810e52d0db101 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 19, 2014, 6:59 a.m.) Review request for mesos and Benjamin Hindman. Changes --- Make lines the right length Repository: mesos-git Description --- Switch path::join() to be variadic Also fix a bug in strings::trim() where when the string was just characters to be trimmed, nothing would be trimmed. Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a24d920c809e36a2a3da1810e52d0db101 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25818/#review53945
---
Bad patch!
Reviews applied: [25789, 25818]
Failed command: make -j3 distcheck
Error:
make dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d mesos-0.21.0; then find mesos-0.21.0 -type d ! -perm -200 -exec
chmod u+w {} ';' rm -rf mesos-0.21.0 || { sleep 5 rm -rf
mesos-0.21.0; }; else :; fi
test -d mesos-0.21.0 || mkdir mesos-0.21.0
(cd 3rdparty make top_distdir=../mesos-0.21.0
distdir=../mesos-0.21.0/3rdparty \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
(cd libprocess make top_distdir=../../mesos-0.21.0
distdir=../../mesos-0.21.0/3rdparty/libprocess \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d ../../mesos-0.21.0/3rdparty/libprocess || mkdir
../../mesos-0.21.0/3rdparty/libprocess
(cd 3rdparty make top_distdir=../../../mesos-0.21.0
distdir=../../../mesos-0.21.0/3rdparty/libprocess/3rdparty \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
(cd stout make top_distdir=../../../../mesos-0.21.0
distdir=../../../../mesos-0.21.0/3rdparty/libprocess/3rdparty/stout \
am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[5]: *** No rule to make target `tests/path.cpp', needed by `distdir'.
Stop.
make[5]: Leaving directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: *** [distdir] Error 1
make[4]: Leaving directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[3]: *** [distdir] Error 1
make[3]: Leaving directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: *** [distdir] Error 1
make[2]: Leaving directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
make: *** [dist] Error 2
- Mesos ReviewBot
On Sept. 19, 2014, 6:59 a.m., Cody Maloney wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25818/
---
(Updated Sept. 19, 2014, 6:59 a.m.)
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
---
Switch path::join() to be variadic
Also fix a bug in strings::trim() where when the string was just characters
to be trimmed, nothing would be trimmed.
Diffs
-
3rdparty/libprocess/3rdparty/Makefile.am
bd1dc8df0259a318a9171a9c045a223800e64f47
3rdparty/libprocess/3rdparty/stout/Makefile.am
2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e
3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp
bc6920a24d920c809e36a2a3da1810e52d0db101
3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd
3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5
Diff: https://reviews.apache.org/r/25818/diff/
Testing
---
make check
Thanks,
Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 19, 2014, 4:08 p.m.) Review request for mesos and Benjamin Hindman. Changes --- Fix build failure, adjust template parameter names and the like to match comments on 25789 Repository: mesos-git Description --- Switch path::join() to be variadic Also fix a bug in strings::trim() where when the string was just characters to be trimmed, nothing would be trimmed. Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a24d920c809e36a2a3da1810e52d0db101 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/#review53965 --- Patch looks great! Reviews applied: [25789, 25818] All tests passed. - Mesos ReviewBot On Sept. 19, 2014, 4:08 p.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 19, 2014, 4:08 p.m.) Review request for mesos and Benjamin Hindman. Repository: mesos-git Description --- Switch path::join() to be variadic Also fix a bug in strings::trim() where when the string was just characters to be trimmed, nothing would be trimmed. Diffs - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a24d920c809e36a2a3da1810e52d0db101 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
Re: Review Request 25818: Switch path::join() to be variadic
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25818/ --- (Updated Sept. 19, 2014, 5:32 a.m.) Review request for mesos and Benjamin Hindman. Changes --- Add missing periods in comments Repository: mesos-git Description --- Switch path::join() to be variadic Also fix a bug in strings::trim() where when the string was just characters to be trimmed, nothing would be trimmed. Diffs (updated) - 3rdparty/libprocess/3rdparty/Makefile.am bd1dc8df0259a318a9171a9c045a223800e64f47 3rdparty/libprocess/3rdparty/stout/Makefile.am 2ee5a0bcc8bef0a5769dafc8ae54aea284993d6e 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp bc6920a24d920c809e36a2a3da1810e52d0db101 3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp a1702cd 3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp PRE-CREATION 3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 51008e5 Diff: https://reviews.apache.org/r/25818/diff/ Testing --- make check Thanks, Cody Maloney
