Re: Review Request 41511: Cleaned up STL I/O includes in public header (including v1).

2016-01-04 Thread Michael Park

---
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  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).

2016-01-04 Thread Michael Park


> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > include/mesos/type_utils.hpp, line 26
> > 
> >
> > 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/#review81
---


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  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).

2015-12-22 Thread Alexander Rukletsov

---
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  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).

2015-12-22 Thread Alexander Rukletsov


> On Dec. 18, 2015, 1:44 p.m., Benjamin Bannier wrote:
> > include/mesos/type_utils.hpp, line 26
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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/#review81
---


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 `` 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).

2015-12-22 Thread Alexander Rukletsov

---
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 `` 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).

2015-12-18 Thread Benjamin Bannier

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41511/#review81
---

Ship it!



include/mesos/type_utils.hpp (line 26)


Looks like a forward decl is good enough here.



include/mesos/v1/mesos.hpp (line 26)


It looks like a forward decl is enough here.



src/common/type_utils.cpp (lines 493 - 494)


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)


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)


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 `` 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
> 
>



Review Request 41511: Cleaned up STL I/O includes in public header (including v1).

2015-12-17 Thread Alexander Rukletsov

---
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 `` 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