Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-03-01 Thread Joris Van Remoortere 

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 2, 2016, 4 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated March 2, 2016, 4 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-03-01 Thread Alex Clemmer 

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

(Updated March 2, 2016, 4 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout:[2/2] Added significant test coverage of `os::rmdir`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
756aa29ed134bf10a645af7f6562f86dc8e488f5 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
3c65d0422dc6e198180d53d1c9e6cb2839137434 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
d0592ef8a774d380e9df66b7e623eb72b29a28b3 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 

Diff: https://reviews.apache.org/r/43908/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-03-01 Thread Alex Clemmer 


> On March 1, 2016, 11:21 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 172
> > 
> >
> > What is this?

THe remnants of a merge of two commits, unfortunately.


- Alex


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


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-03-01 Thread Alex Clemmer 


> On March 1, 2016, 11:21 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/Makefile.am, line 48
> > 
> >
> > style

I'd just like to clarify that I made this line up in `git diff` because it was 
off... I guess git is subtly different in how it presents diffs.


> On March 1, 2016, 11:21 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, lines 214-297
> > 
> >
> > I don't understand why these got removed.
> > They are not covered by the tests your introduced.

These tests are completely redundant. They exist in both `filesystem_tests.cpp` 
and `os_tests.cpp`. So I've removed them from `os.hpp`.


- Alex


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


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-03-01 Thread Joris Van Remoortere 

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




3rdparty/libprocess/3rdparty/stout/Makefile.am (line 48)


style



3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp (line 32)


We have `hashset::EMPTY`.
`const hashset& EMPTY = hashset::EMPTY;`



3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp (lines 51 - 91)


Can you add a TODO to parameterize this test?
It's the same code copy pasted with 1 difference: relative vs. absolute 
path.



3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp (line 172)


What is this?



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp (lines 163 - 194)


I don't understand why these got removed.
They are not covered by the tests your introduced.


- Joris Van Remoortere


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-02-26 Thread Alex Naparu 

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


Ship it!




Ship It!

- Alex Naparu


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-02-25 Thread Mesos ReviewBot 

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



Bad patch!

Reviews applied: [43908, 43907, 43906]

Failed command: ./support/apply-review.sh -n -r 43906

Error:
2016-02-26 05:06:40 URL:https://reviews.apache.org/r/43906/diff/raw/ 
[3197/3197] -> "43906.patch" [1]
error: patch failed: src/CMakeLists.txt:223
error: src/CMakeLists.txt: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/11655/console

- Mesos ReviewBot


On Feb. 25, 2016, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43908/
> ---
> 
> (Updated Feb. 25, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout:[2/2] Added significant test coverage of `os::rmdir`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 756aa29ed134bf10a645af7f6562f86dc8e488f5 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 3c65d0422dc6e198180d53d1c9e6cb2839137434 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> d0592ef8a774d380e9df66b7e623eb72b29a28b3 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 
> 
> Diff: https://reviews.apache.org/r/43908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 43908: Stout:[2/2] Added significant test coverage of `os::rmdir`.

2016-02-25 Thread Alex Clemmer 

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout:[2/2] Added significant test coverage of `os::rmdir`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/Makefile.am 
756aa29ed134bf10a645af7f6562f86dc8e488f5 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
3c65d0422dc6e198180d53d1c9e6cb2839137434 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
d0592ef8a774d380e9df66b7e623eb72b29a28b3 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
a2bc5c40167896a3df2cfb5b1f3cf58c20ea1422 

Diff: https://reviews.apache.org/r/43908/diff/


Testing
---


Thanks,

Alex Clemmer