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/03/20 09:48:06 UTC
Review Request 70249: Made hashmap::containsValue consistent with
stout naming convention.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70249/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Benno Evers.
Repository: mesos
Description
-------
In stout we use snake case, not camel case.
Diffs
-----
3rdparty/stout/include/stout/hashmap.hpp 61bf80ea90b97c0aed19f21ac0d258ff370357b6
3rdparty/stout/tests/hashmap_tests.cpp 38a33dc71eb3fce1c7be036c36f4791e86511f1e
src/master/master.cpp dc68fc324de7242737123015fbac19a2129778ce
src/slave/containerizer/docker.cpp fc29e20bedd37f77b9be49cedbb1012473abdfee
src/slave/containerizer/mesos/launcher.cpp b38f88db4d120eb27ad56bdb7639cb80c76063d3
Diff: https://reviews.apache.org/r/70249/diff/1/
Testing
-------
`make check`, not a functional change
Thanks,
Benjamin Bannier
Re: Review Request 70249: Made hashmap::containsValue consistent with
stout naming convention.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70249/#review213828
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['70249']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2984/mesos-review-70249
Relevant logs:
- [mesos-tests-cmake.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2984/mesos-review-70249/logs/mesos-tests-cmake.log):
```
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501): warning C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479): warning C4101: 'addrstr': unreferenced local variable [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\recordio.c(170): warning C4267: '=': conversion from 'size_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\mt_adaptor.c(496): warning C4244: '=': conversion from 'time_t' to 'int32_t', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256): warning C4090: 'function': different 'const' qualifiers [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166): warning C4716: 'pthread_cond_broadcast': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205): warning C4716: 'pthread_cond_wait': must return a value [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548): warning C4996: 'fopen': This function or variable may be unsafe. Consider using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569): warning C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
"D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"D:\DCOS\mesos\src\mesos.vcxproj" (default target) (20) ->
(ClCompile target) ->
d:\dcos\mesos\mesos\src\slave\containerizer\mesos\launcher.cpp(64): error C2039: 'containsValue': is not a member of 'hashmap<mesos::ContainerID,pid_t,std::hash<mesos::ContainerID>,std::equal_to<Elem>>' [D:\DCOS\mesos\src\mesos.vcxproj]
d:\dcos\mesos\mesos\src\master\master.cpp(10844): error C2039: 'containsValue': is not a member of 'hashmap<process::UPID,Option<T>,std::hash<process::UPID>,std::equal_to<Elem>>' [D:\DCOS\mesos\src\mesos.vcxproj]
172 Warning(s)
2 Error(s)
Time Elapsed 00:09:28.08
```
- Mesos Reviewbot Windows
On March 20, 2019, 10:48 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70249/
> -----------------------------------------------------------
>
> (Updated March 20, 2019, 10:48 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Benno Evers.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In stout we use snake case, not camel case.
>
> We will clean up users outside of stout in a follow-up patch.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/hashmap.hpp 61bf80ea90b97c0aed19f21ac0d258ff370357b6
> 3rdparty/stout/tests/hashmap_tests.cpp 38a33dc71eb3fce1c7be036c36f4791e86511f1e
>
>
> Diff: https://reviews.apache.org/r/70249/diff/2/
>
>
> Testing
> -------
>
> `make check`, not a functional change
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 70249: Made hashmap::containsValue consistent with
stout naming convention.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70249/#review213826
-----------------------------------------------------------
Ship it!
I agree with the change; `hashmap` extends the interface of `std::unordered_map` and thus should continue to use snake_case for consistency.
I don't quite agree with the commit message. While it is correct de jure that we should use snake_case in stout and libprocess, I think de facto >90% of the interactions with these libraries in Mesos code goes through camelCase interfaces (i.e. `Option::isSome()`, `Future::isReady()`) so in general it would be better to update the style guide to reflect this reality.
- Benno Evers
On March 20, 2019, 9:48 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70249/
> -----------------------------------------------------------
>
> (Updated March 20, 2019, 9:48 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Benno Evers.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In stout we use snake case, not camel case.
>
> We will clean up users outside of stout in a follow-up patch.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/hashmap.hpp 61bf80ea90b97c0aed19f21ac0d258ff370357b6
> 3rdparty/stout/tests/hashmap_tests.cpp 38a33dc71eb3fce1c7be036c36f4791e86511f1e
>
>
> Diff: https://reviews.apache.org/r/70249/diff/2/
>
>
> Testing
> -------
>
> `make check`, not a functional change
>
>
> Thanks,
>
> Benjamin Bannier
>
>