You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2015/12/17 15:37:18 UTC
Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
Bugs: MESOS-4183
https://issues.apache.org/jira/browse/MESOS-4183
Repository: mesos
Description
-------
Where possible move `operator<<` definitions and functions using streams to ".cpp" files and include `<iosfwd>` in ".hpp"s. Also remove unused I/O includes, add necessary includes, clean up `std::` prefixes and blank lines.
Diffs
-----
include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211
src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717
Diff: https://reviews.apache.org/r/41511/diff/
Testing
-------
make check on Mac OS 10.10.4.
Thanks,
Alexander Rukletsov
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/#review112534
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On Dec. 22, 2015, 5:55 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41511/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2015, 5:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
>
>
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Where possible move operator<< definitions and functions using streams
> to ".cpp" files and include <iosfwd> in ".hpp"s. Also remove unused I/O
> includes, add necessary includes, clean up `std::` prefixes and blank
> lines.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
> include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
> include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
> include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
> include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
> include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
> src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3
> src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
> src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
> src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764
>
> Diff: https://reviews.apache.org/r/41511/diff/
>
>
> Testing
> -------
>
> make check on Mac OS 10.10.4.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/
-----------------------------------------------------------
(Updated Dec. 22, 2015, 5:55 p.m.)
Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
Changes
-------
Wrapped description.
Bugs: MESOS-4183
https://issues.apache.org/jira/browse/MESOS-4183
Repository: mesos
Description (updated)
-------
Where possible move operator<< definitions and functions using streams
to ".cpp" files and include <iosfwd> in ".hpp"s. Also remove unused I/O
includes, add necessary includes, clean up `std::` prefixes and blank
lines.
Diffs
-----
include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3
src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764
Diff: https://reviews.apache.org/r/41511/diff/
Testing
-------
make check on Mac OS 10.10.4.
Thanks,
Alexander Rukletsov
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/
-----------------------------------------------------------
(Updated Dec. 22, 2015, 5:52 p.m.)
Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
Changes
-------
Addressed comments.
Bugs: MESOS-4183
https://issues.apache.org/jira/browse/MESOS-4183
Repository: mesos
Description
-------
Where possible move `operator<<` definitions and functions using streams to ".cpp" files and include `<iosfwd>` in ".hpp"s. Also remove unused I/O includes, add necessary includes, clean up `std::` prefixes and blank lines.
Diffs (updated)
-----
include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3
src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764
Diff: https://reviews.apache.org/r/41511/diff/
Testing
-------
make check on Mac OS 10.10.4.
Thanks,
Alexander Rukletsov
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > include/mesos/type_utils.hpp, line 26
> > <https://reviews.apache.org/r/41511/diff/1/?file=1168654#file1168654line26>
> >
> > Looks like a forward decl is good enough here.
I'm not sure we can forward declare it since it's a template.
> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > include/mesos/v1/mesos.hpp, line 26
> > <https://reviews.apache.org/r/41511/diff/1/?file=1168655#file1168655line26>
> >
> > It looks like a forward decl is enough here.
See above.
> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > src/v1/resources.cpp, line 23
> > <https://reviews.apache.org/r/41511/diff/1/?file=1168662#file1168662line23>
> >
> > Do you want to add an include/forward decl for RepeatedFieldPtr as well?
Good catch! I will add it into both hpp and cpp since we tend not to use transient includes.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/#review111181
-----------------------------------------------------------
On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41511/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:37 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
>
>
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Where possible move `operator<<` definitions and functions using streams to ".cpp" files and include `<iosfwd>` in ".hpp"s. Also remove unused I/O includes, add necessary includes, clean up `std::` prefixes and blank lines.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
> include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
> include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
> include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
> include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
> include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
> src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211
> src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
> src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
> src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717
>
> Diff: https://reviews.apache.org/r/41511/diff/
>
>
> Testing
> -------
>
> make check on Mac OS 10.10.4.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Michael Park <mp...@apache.org>.
> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > include/mesos/type_utils.hpp, line 26
> > <https://reviews.apache.org/r/41511/diff/1/?file=1168654#file1168654line26>
> >
> > Looks like a forward decl is good enough here.
>
> Alexander Rukletsov wrote:
> I'm not sure we can forward declare it since it's a template.
As AlexR pointed out, the `operator<<` being defined is indeed a template and we therefore do need the full definition of `RepeatedPtrField`.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/#review111181
-----------------------------------------------------------
On Dec. 22, 2015, 5:55 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41511/
> -----------------------------------------------------------
>
> (Updated Dec. 22, 2015, 5:55 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
>
>
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Where possible move operator<< definitions and functions using streams
> to ".cpp" files and include <iosfwd> in ".hpp"s. Also remove unused I/O
> includes, add necessary includes, clean up `std::` prefixes and blank
> lines.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
> include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
> include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
> include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
> include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
> include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
> src/common/resources.cpp b9d31aef8babee212374e352c57fadbff02167f3
> src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
> src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
> src/v1/resources.cpp 0c0eee6b3b7ae3254d7dc7b06187855c9b873764
>
> Diff: https://reviews.apache.org/r/41511/diff/
>
>
> Testing
> -------
>
> make check on Mac OS 10.10.4.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 41511: Cleaned up STL I/O includes in public header
(including v1).
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/#review111181
-----------------------------------------------------------
Ship it!
include/mesos/type_utils.hpp (line 26)
<https://reviews.apache.org/r/41511/#comment171282>
Looks like a forward decl is good enough here.
include/mesos/v1/mesos.hpp (line 26)
<https://reviews.apache.org/r/41511/#comment171281>
It looks like a forward decl is enough here.
src/common/type_utils.cpp (lines 493 - 494)
<https://reviews.apache.org/r/41511/#comment171284>
Not yours, but the general pattern for this here seems to be reuse the temp, `return stream << stringify(map)`.
src/v1/mesos.cpp (lines 488 - 489)
<https://reviews.apache.org/r/41511/#comment171285>
Not yours, but the general pattern for this here seems to be to reuse the temp, `return stream << stringify(map)`.
src/v1/resources.cpp (line 23)
<https://reviews.apache.org/r/41511/#comment171286>
Do you want to add an include/forward decl for RepeatedFieldPtr as well?
- Benjamin Bannier
On Dec. 17, 2015, 2:37 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41511/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2015, 2:37 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, and Michael Park.
>
>
> Bugs: MESOS-4183
> https://issues.apache.org/jira/browse/MESOS-4183
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Where possible move `operator<<` definitions and functions using streams to ".cpp" files and include `<iosfwd>` in ".hpp"s. Also remove unused I/O includes, add necessary includes, clean up `std::` prefixes and blank lines.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9c8bfb0e7e1e40ca99b4cc75079e5b950630243d
> include/mesos/type_utils.hpp b370b5180e71ed246fa09bf0fe119dfa946d0b08
> include/mesos/v1/mesos.hpp 80e76040de77e65b20f10b0465124ab86c1feab6
> include/mesos/v1/resources.hpp db87c992e2ec466cb1d4d17775bf6423a2fb15aa
> include/mesos/v1/values.hpp a160bd4d49d53fd2f794f36fea69ea99a455af09
> include/mesos/values.hpp 58af972186d7156660eac742b41501ce420cefe9
> src/common/resources.cpp 5a7981744726a0544435cbbd5007487a43a01211
> src/common/type_utils.cpp c6c9ba7d1bfc6e6202b9e3ff94a74711e3602d6f
> src/v1/mesos.cpp 13a58baf086ef3aaf46abcf7f58b71b7e2639728
> src/v1/resources.cpp d300842576b2fb8fe75f5b18699f23c1fd324717
>
> Diff: https://reviews.apache.org/r/41511/diff/
>
>
> Testing
> -------
>
> make check on Mac OS 10.10.4.
>
>
> Thanks,
>
> Alexander Rukletsov
>
>