You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alex Clemmer <cl...@gmail.com> on 2016/02/10 07:41:17 UTC
Review Request 43411: Windows: Added dynamic library loading tests to
build.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/
-----------------------------------------------------------
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Repository: mesos
Description
-------
Windows: Added dynamic library loading tests to build.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
Diff: https://reviews.apache.org/r/43411/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by M Lawindi <ml...@microsoft.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review118908
-----------------------------------------------------------
Ship it!
Ship It!
- M Lawindi
On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:47 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Daniel Pravat <dp...@outlook.com>.
> On Feb. 11, 2016, 6 p.m., Daniel Pravat wrote:
> > Ship It!
RB diff is showing a more complicated picture.
- Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review118889
-----------------------------------------------------------
On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:47 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Daniel Pravat <dp...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review118889
-----------------------------------------------------------
Ship it!
Ship It!
- Daniel Pravat
On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:47 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Feb. 10, 2016, 11:50 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> >
> > Reviews applied: [43407, 43409, 43410]
> >
> > Failed command: ./support/apply-review.sh -n -r 43410
> >
> > Error:
> > 2016-02-10 11:49:59 URL:https://reviews.apache.org/r/43410/diff/raw/ [8010/8010] -> "43410.patch" [1]
> > 3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
> > 3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
> > Total errors found: 2
> > Checking 3 files
> >
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11361/console
Ah, sorry, this patch is extremely old, and hhas been resurrected. I'll fix it.
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review118629
-----------------------------------------------------------
On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:47 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review118629
-----------------------------------------------------------
Bad patch!
Reviews applied: [43407, 43409, 43410]
Failed command: ./support/apply-review.sh -n -r 43410
Error:
2016-02-10 11:49:59 URL:https://reviews.apache.org/r/43410/diff/raw/ [8010/8010] -> "43410.patch" [1]
3rdparty/libprocess/3rdparty/stout/include/stout/posix/dynamiclibrary.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
3rdparty/libprocess/3rdparty/stout/include/stout/windows/dynamiclibrary.hpp:1: A license header should appear on the file's first line starting with '// Licensed'.: /**
Total errors found: 2
Checking 3 files
Full log: https://builds.apache.org/job/mesos-reviewbot/11361/console
- Mesos ReviewBot
On Feb. 10, 2016, 6:47 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 6:47 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Naparu <al...@outlook.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review119929
-----------------------------------------------------------
Ship it!
Ship It!
- Alex Naparu
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/
-----------------------------------------------------------
(Updated Feb. 25, 2016, 5:15 p.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Windows: Added dynamic library loading tests to build.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
Diff: https://reviews.apache.org/r/43411/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
> On Feb. 25, 2016, 12:09 a.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp, lines 73-74
> > <https://reviews.apache.org/r/43411/diff/2/?file=1253408#file1253408line73>
> >
> > We're only testing that `loadSymbol` fails if we don't call `open` first, not `close`. Right? Seems like this was copy/pasted from above test.
Yep. I copied it. My bad. :(
- Alex
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review120616
-----------------------------------------------------------
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/#review120616
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp (line 54)
<https://reviews.apache.org/r/43411/#comment182083>
Let's we pull this string out as `invalid_symbol` and just use `"InvalidSymbol"`
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp (lines 69 - 70)
<https://reviews.apache.org/r/43411/#comment182084>
We're only testing that `loadSymbol` fails if we don't call `open` first, not `close`. Right? Seems like this was copy/pasted from above test.
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp (line 92)
<https://reviews.apache.org/r/43411/#comment182085>
Let's also take this out to `invalid_library_path` and call it `"InvalidLibraryPath"`
- Michael Park
On Feb. 18, 2016, 12:59 a.m., Alex Clemmer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43411/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2016, 12:59 a.m.)
>
>
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
>
>
> Bugs: MESOS-4496
> https://issues.apache.org/jira/browse/MESOS-4496
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Windows: Added dynamic library loading tests to build.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
> 3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
>
> Diff: https://reviews.apache.org/r/43411/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Alex Clemmer
>
>
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/
-----------------------------------------------------------
(Updated Feb. 18, 2016, 12:59 a.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Windows: Added dynamic library loading tests to build.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
Diff: https://reviews.apache.org/r/43411/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 6:47 a.m.)
Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
Bugs: MESOS-4496
https://issues.apache.org/jira/browse/MESOS-4496
Repository: mesos
Description
-------
Windows: Added dynamic library loading tests to build.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
Diff: https://reviews.apache.org/r/43411/diff/
Testing
-------
Thanks,
Alex Clemmer
Re: Review Request 43411: Windows: Added dynamic library loading
tests to build.
Posted by Alex Clemmer <cl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43411/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 6:44 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
-------
Windows: Added dynamic library loading tests to build.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 3c65d0422dc6e198180d53d1c9e6cb2839137434
3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp 27626ae28db090f1a002239ff5c674b82e8fc9a8
Diff: https://reviews.apache.org/r/43411/diff/
Testing
-------
Thanks,
Alex Clemmer