Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-20 Thread Jan Schlicht


> On Jan. 11, 2016, 4:53 p.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/include/process/http.hpp, line 538
> > 
> >
> > Instead of using `std::vector`, we could also use 
> > `std::initializer_list` here. Same for line 527.
> 
> Alexander Rojas wrote:
> So, as we discussed using vector allows you to use initializer lists, but 
> if you build the vector ourside you cannot pass it as a parameters, so you 
> end up with less capabilities instead of more.

Yes, that makes sense. Thanks for the explanation.


- Jan


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


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-20 Thread Alexander Rojas


> On Jan. 11, 2016, 4:53 p.m., Jan Schlicht wrote:
> > 3rdparty/libprocess/include/process/http.hpp, line 538
> > 
> >
> > Instead of using `std::vector`, we could also use 
> > `std::initializer_list` here. Same for line 527.

So, as we discussed using vector allows you to use initializer lists, but if 
you build the vector ourside you cannot pass it as a parameters, so you end up 
with less capabilities instead of more.


- Alexander


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


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-20 Thread Till Toenshoff

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

Ship it!


Ship It!

- Till Toenshoff


On Jan. 11, 2016, 3:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-11 Thread Jan Schlicht

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

Ship it!



3rdparty/libprocess/include/process/http.hpp (line 538)


Instead of using `std::vector`, we could also use `std::initializer_list` 
here. Same for line 527.


- Jan Schlicht


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-11 Thread Alexander Rojas


> On Jan. 11, 2016, 4:24 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/include/process/http.hpp, lines 527-546
> > 
> >
> > Sorry for not noticing this earlier, but the constructors are now 
> > almost identical and we might just use e.g., the second one if we give a 
> > default arg to `body` (probably `Status::string(Status::UNAUTHORIZED)`) and 
> > make it `explicit`.

We don't seem to do that in any of the constructors here.


- Alexander


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


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-11 Thread Benjamin Bannier

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

Ship it!


Ship It!

- Benjamin Bannier


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 42144: Removed deprecated constructor of http::Unauthorized in libprocess.

2016-01-11 Thread Benjamin Bannier

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



3rdparty/libprocess/include/process/http.hpp (lines 527 - 546)


Sorry for not noticing this earlier, but the constructors are now almost 
identical and we might just use e.g., the second one if we give a default arg 
to `body` (probably `Status::string(Status::UNAUTHORIZED)`) and make it 
`explicit`.


- Benjamin Bannier


On Jan. 11, 2016, 4:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42144/
> ---
> 
> (Updated Jan. 11, 2016, 4:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Ben Mahler, Jan Schlicht, and 
> Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The constructor of `process::http::Unauthorized(const std::string&)` is 
> marked as deprecated. This patch fully removes the constructor and cleans up 
> its usage in the libprocess codebase.
> 
> This change also allows to use initializer lists on the 
> `process::http::Unauthorized(const std::vector&)` constructor 
> since there is no longer an ambiguity.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> ed708fe4b0006782a19f9c61603f152e32a02e8e 
>   3rdparty/libprocess/src/authenticator.cpp 
> 7371a62ecb2b3a42cb5ff5fb78bc4bcdb22875c4 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> e5999d8b49937a17033482c21536edb5c10420e6 
> 
> Diff: https://reviews.apache.org/r/42144/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>