You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/12/20 03:43:41 UTC
Review Request 64735: Fixed conversion warnings in tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64735/
-----------------------------------------------------------
Review request for mesos, Greg Mann, Jie Yu, Joseph Wu, and Michael Park.
Repository: mesos
Description
-------
Fixed `double -> float` conversions. While the Protobuf type and literal
type are both `double`, the `FLOAT_EQ` tests were being used instead of
`DOUBLE_EQ`, causing a conversion warning.
Fixed `size_t` to `int` conversion warnings by casting.
Fixed warning for `Seconds(double)` as it takes `int64_t`. Can't change
the argument types because they're set in the protocol.
Diffs
-----
src/tests/attributes_tests.cpp f053292e9185e9709d4da9c0b462541691772f1b
src/tests/protobuf_io_tests.cpp 4a2e3a3bc87c1a3394368439e5dd30b410e468a4
src/tests/resource_offers_tests.cpp 5564636dbabe25801a12b613a1cb04cd52ebb458
src/tests/resources_tests.cpp 64bde85e0e7f0bd395189eb6a8635383095b2f07
src/tests/resources_utils.cpp 9736c2a090250562704f2c874ce3872be68555ed
src/tests/scheduler_tests.cpp 87ef589fc296d01cfca8bf0b67390b097f68406b
src/tests/slave_tests.cpp b87015d4b3f3fcd1948978d42e20de8108e8f93a
src/tests/values_tests.cpp f7e6fa786b85460185dbf83572e5d282671ab519
Diff: https://reviews.apache.org/r/64735/diff/1/
Testing
-------
These are in Mesos.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 64735: Fixed conversion warnings in tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
> On Jan. 5, 2018, 3:26 p.m., Joseph Wu wrote:
> > src/tests/protobuf_io_tests.cpp
> > Line 161 (original), 161 (patched)
> > <https://reviews.apache.org/r/64735/diff/2/?file=1932994#file1932994line161>
> >
> > Both `expected` and `actual` are the same type here, so why does only `actual` need the cast?
You're right, they both needed it. Easy to miss because the warning isn't generated until the file's been touched and ergo rebuilt, such a slow process :P
I changed it so that the iterator and comparator are both `int`, since that's the expected type of `Get(int)`, and this eliminated needing to static cast.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64735/#review194917
-----------------------------------------------------------
On Jan. 5, 2018, 1:54 p.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64735/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2018, 1:54 p.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Joseph Wu, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed `double -> float` conversions. While the Protobuf type and literal
> type are both `double`, the `FLOAT_EQ` tests were being used instead of
> `DOUBLE_EQ`, causing a conversion warning.
>
> Fixed `size_t` to `int` conversion warnings by casting.
>
> Fixed warning for `Seconds(double)` as it takes `int64_t`. Can't change
> the argument types because they're set in the protocol.
>
>
> Diffs
> -----
>
> src/tests/attributes_tests.cpp f053292e9185e9709d4da9c0b462541691772f1b
> src/tests/protobuf_io_tests.cpp 4a2e3a3bc87c1a3394368439e5dd30b410e468a4
> src/tests/resource_offers_tests.cpp 5564636dbabe25801a12b613a1cb04cd52ebb458
> src/tests/resources_tests.cpp bd328b254eff3815fa2e85d17179cf593cb8a349
> src/tests/resources_utils.cpp 9736c2a090250562704f2c874ce3872be68555ed
> src/tests/scheduler_tests.cpp 87ef589fc296d01cfca8bf0b67390b097f68406b
> src/tests/slave_tests.cpp 159192053087e971746943a1bc17a76143a2d9af
> src/tests/values_tests.cpp f7e6fa786b85460185dbf83572e5d282671ab519
>
>
> Diff: https://reviews.apache.org/r/64735/diff/3/
>
>
> Testing
> -------
>
> These are in Mesos.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 64735: Fixed conversion warnings in tests.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64735/#review194917
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
src/tests/protobuf_io_tests.cpp
Line 161 (original), 161 (patched)
<https://reviews.apache.org/r/64735/#comment273997>
Both `expected` and `actual` are the same type here, so why does only `actual` need the cast?
- Joseph Wu
On Jan. 5, 2018, 1:54 p.m., Andrew Schwartzmeyer wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64735/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2018, 1:54 p.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Joseph Wu, and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed `double -> float` conversions. While the Protobuf type and literal
> type are both `double`, the `FLOAT_EQ` tests were being used instead of
> `DOUBLE_EQ`, causing a conversion warning.
>
> Fixed `size_t` to `int` conversion warnings by casting.
>
> Fixed warning for `Seconds(double)` as it takes `int64_t`. Can't change
> the argument types because they're set in the protocol.
>
>
> Diffs
> -----
>
> src/tests/attributes_tests.cpp f053292e9185e9709d4da9c0b462541691772f1b
> src/tests/protobuf_io_tests.cpp 4a2e3a3bc87c1a3394368439e5dd30b410e468a4
> src/tests/resource_offers_tests.cpp 5564636dbabe25801a12b613a1cb04cd52ebb458
> src/tests/resources_tests.cpp bd328b254eff3815fa2e85d17179cf593cb8a349
> src/tests/resources_utils.cpp 9736c2a090250562704f2c874ce3872be68555ed
> src/tests/scheduler_tests.cpp 87ef589fc296d01cfca8bf0b67390b097f68406b
> src/tests/slave_tests.cpp 159192053087e971746943a1bc17a76143a2d9af
> src/tests/values_tests.cpp f7e6fa786b85460185dbf83572e5d282671ab519
>
>
> Diff: https://reviews.apache.org/r/64735/diff/2/
>
>
> Testing
> -------
>
> These are in Mesos.
>
>
> Thanks,
>
> Andrew Schwartzmeyer
>
>
Re: Review Request 64735: Fixed conversion warnings in tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64735/
-----------------------------------------------------------
(Updated Jan. 11, 2018, 5:03 p.m.)
Review request for mesos, Greg Mann, Jie Yu, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Repository: mesos
Description
-------
Fixed `double -> float` conversions. While the Protobuf type and literal
type are both `double`, the `FLOAT_EQ` tests were being used instead of
`DOUBLE_EQ`, causing a conversion warning.
Fixed `size_t` to `int` conversion warnings by casting.
Fixed warning for `Seconds(double)` as it takes `int64_t`. Can't change
the argument types because they're set in the protocol.
Diffs (updated)
-----
src/tests/attributes_tests.cpp f053292e9185e9709d4da9c0b462541691772f1b
src/tests/protobuf_io_tests.cpp ddbda036ac78e38a511802c921a48469bacb1cd5
src/tests/resource_offers_tests.cpp 5564636dbabe25801a12b613a1cb04cd52ebb458
src/tests/resources_tests.cpp bd328b254eff3815fa2e85d17179cf593cb8a349
src/tests/resources_utils.cpp 9736c2a090250562704f2c874ce3872be68555ed
src/tests/scheduler_tests.cpp 87ef589fc296d01cfca8bf0b67390b097f68406b
src/tests/slave_tests.cpp 288a41932d7603f9560a19e1310e5368a35cc948
src/tests/values_tests.cpp f7e6fa786b85460185dbf83572e5d282671ab519
Diff: https://reviews.apache.org/r/64735/diff/4/
Changes: https://reviews.apache.org/r/64735/diff/3-4/
Testing
-------
These are in Mesos.
Thanks,
Andrew Schwartzmeyer
Re: Review Request 64735: Fixed conversion warnings in tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64735/
-----------------------------------------------------------
(Updated Jan. 5, 2018, 1:54 p.m.)
Review request for mesos, Greg Mann, Jie Yu, Joseph Wu, and Michael Park.
Changes
-------
Rebased.
Repository: mesos
Description
-------
Fixed `double -> float` conversions. While the Protobuf type and literal
type are both `double`, the `FLOAT_EQ` tests were being used instead of
`DOUBLE_EQ`, causing a conversion warning.
Fixed `size_t` to `int` conversion warnings by casting.
Fixed warning for `Seconds(double)` as it takes `int64_t`. Can't change
the argument types because they're set in the protocol.
Diffs (updated)
-----
src/tests/attributes_tests.cpp f053292e9185e9709d4da9c0b462541691772f1b
src/tests/protobuf_io_tests.cpp 4a2e3a3bc87c1a3394368439e5dd30b410e468a4
src/tests/resource_offers_tests.cpp 5564636dbabe25801a12b613a1cb04cd52ebb458
src/tests/resources_tests.cpp bd328b254eff3815fa2e85d17179cf593cb8a349
src/tests/resources_utils.cpp 9736c2a090250562704f2c874ce3872be68555ed
src/tests/scheduler_tests.cpp 87ef589fc296d01cfca8bf0b67390b097f68406b
src/tests/slave_tests.cpp 159192053087e971746943a1bc17a76143a2d9af
src/tests/values_tests.cpp f7e6fa786b85460185dbf83572e5d282671ab519
Diff: https://reviews.apache.org/r/64735/diff/2/
Changes: https://reviews.apache.org/r/64735/diff/1-2/
Testing
-------
These are in Mesos.
Thanks,
Andrew Schwartzmeyer