You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2019/01/11 10:02:25 UTC
Review Request 69718: Avoided copying some `Owned` values.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69718/
-----------------------------------------------------------
Review request for mesos and Jan Schlicht.
Repository: mesos
Description
-------
The resource provider manager's `start` method takes ownership of a
passed `Owned` value. Since `Owned` values are not shared (but owned),
they cannot be copied (at least semantically until MESOS-5122 is
implemented).
This patch fixes existing code to make sure we do not copy `Owned`
values when invoking `ResourceProviderManager::start`. This should be a
non-functional change modulo existing bugs.
Diffs
-----
src/tests/api_tests.cpp c597243e2e210e83a4ab7441fbcfa3198b43d849
src/tests/master_slave_reconciliation_tests.cpp 979048c1e90966244135f149ff5ef66054f27d0d
src/tests/master_tests.cpp 67713c81953d7ed90385ffbcfd7804de575c8977
src/tests/operation_reconciliation_tests.cpp a5e81a1d50ea6135932610d5e4c45e87e9354a09
src/tests/resource_provider_manager_tests.cpp 20cfb340bf634354865d79c92ee70cddca08f28a
src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4
Diff: https://reviews.apache.org/r/69718/diff/1/
Testing
-------
`make check`
`./support/mesos-tidy.sh`
Thanks,
Benjamin Bannier
Re: Review Request 69718: Avoided copying some `Owned` values.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69718/#review211877
-----------------------------------------------------------
PASS: Mesos patch 69718 was successfully built and tested.
Reviews applied: `['69718']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2759/mesos-review-69718
- Mesos Reviewbot Windows
On Jan. 11, 2019, 10:17 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69718/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 10:17 a.m.)
>
>
> Review request for mesos and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The resource provider manager's `start` method takes ownership of a
> passed `Owned` value. Since `Owned` values are not shared (but owned),
> they cannot be copied (at least semantically until MESOS-5122 is
> implemented).
>
> This patch fixes existing code to make sure we do not copy `Owned`
> values when invoking `ResourceProviderManager::start`. This should be a
> non-functional change modulo existing bugs.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp c597243e2e210e83a4ab7441fbcfa3198b43d849
> src/tests/master_slave_reconciliation_tests.cpp 979048c1e90966244135f149ff5ef66054f27d0d
> src/tests/master_tests.cpp 67713c81953d7ed90385ffbcfd7804de575c8977
> src/tests/operation_reconciliation_tests.cpp a5e81a1d50ea6135932610d5e4c45e87e9354a09
> src/tests/resource_provider_manager_tests.cpp 20cfb340bf634354865d79c92ee70cddca08f28a
> src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4
>
>
> Diff: https://reviews.apache.org/r/69718/diff/2/
>
>
> Testing
> -------
>
> `make check`
> `./support/mesos-tidy.sh`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 69718: Avoided copying some `Owned` values.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69718/
-----------------------------------------------------------
(Updated Jan. 11, 2019, 11:17 a.m.)
Review request for mesos and Jan Schlicht.
Changes
-------
Fixed include mutilation
Repository: mesos
Description
-------
The resource provider manager's `start` method takes ownership of a
passed `Owned` value. Since `Owned` values are not shared (but owned),
they cannot be copied (at least semantically until MESOS-5122 is
implemented).
This patch fixes existing code to make sure we do not copy `Owned`
values when invoking `ResourceProviderManager::start`. This should be a
non-functional change modulo existing bugs.
Diffs (updated)
-----
src/tests/api_tests.cpp c597243e2e210e83a4ab7441fbcfa3198b43d849
src/tests/master_slave_reconciliation_tests.cpp 979048c1e90966244135f149ff5ef66054f27d0d
src/tests/master_tests.cpp 67713c81953d7ed90385ffbcfd7804de575c8977
src/tests/operation_reconciliation_tests.cpp a5e81a1d50ea6135932610d5e4c45e87e9354a09
src/tests/resource_provider_manager_tests.cpp 20cfb340bf634354865d79c92ee70cddca08f28a
src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4
Diff: https://reviews.apache.org/r/69718/diff/2/
Changes: https://reviews.apache.org/r/69718/diff/1-2/
Testing
-------
`make check`
`./support/mesos-tidy.sh`
Thanks,
Benjamin Bannier
Re: Review Request 69718: Avoided copying some `Owned` values.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69718/#review211876
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/master_slave_reconciliation_tests.cpp
Line 17 (original), 17-18 (patched)
<https://reviews.apache.org/r/69718/#comment297457>
Wrong sorting.
src/tests/master_tests.cpp
Lines 17-30 (original), 17-30 (patched)
<https://reviews.apache.org/r/69718/#comment297456>
This looks odd. Wouldn't you want to just insert `<utility>` here?
src/tests/resource_provider_manager_tests.cpp
Lines 19 (patched)
<https://reviews.apache.org/r/69718/#comment297458>
Double `<vector>` instead of `<utility>` before `<vector>`.
- Jan Schlicht
On Jan. 11, 2019, 11:02 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69718/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2019, 11:02 a.m.)
>
>
> Review request for mesos and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The resource provider manager's `start` method takes ownership of a
> passed `Owned` value. Since `Owned` values are not shared (but owned),
> they cannot be copied (at least semantically until MESOS-5122 is
> implemented).
>
> This patch fixes existing code to make sure we do not copy `Owned`
> values when invoking `ResourceProviderManager::start`. This should be a
> non-functional change modulo existing bugs.
>
>
> Diffs
> -----
>
> src/tests/api_tests.cpp c597243e2e210e83a4ab7441fbcfa3198b43d849
> src/tests/master_slave_reconciliation_tests.cpp 979048c1e90966244135f149ff5ef66054f27d0d
> src/tests/master_tests.cpp 67713c81953d7ed90385ffbcfd7804de575c8977
> src/tests/operation_reconciliation_tests.cpp a5e81a1d50ea6135932610d5e4c45e87e9354a09
> src/tests/resource_provider_manager_tests.cpp 20cfb340bf634354865d79c92ee70cddca08f28a
> src/tests/slave_tests.cpp 4aed5d68e9a408821880ffaede482937be1999f4
>
>
> Diff: https://reviews.apache.org/r/69718/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `./support/mesos-tidy.sh`
>
>
> Thanks,
>
> Benjamin Bannier
>
>