You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kevin Klues <kl...@gmail.com> on 2016/02/04 21:30:20 UTC
Re: Review Request 42957: Added remove() calls to process::Help.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/
-----------------------------------------------------------
(Updated Feb. 4, 2016, 8:30 p.m.)
Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
Changes
-------
Met with bmahler offline to make some improvements on this.
Bugs: MESOS-4552
https://issues.apache.org/jira/browse/MESOS-4552
Repository: mesos
Description
-------
Previously, there was no way to remove an installed help
string for a given endpoint.
This commit adds the ability to remove these help strings.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
Diff: https://reviews.apache.org/r/42957/diff/
Testing
-------
Thanks,
Kevin Klues
Re: Review Request 42957: Added remove() calls to process::Help.
Posted by Kevin Klues <kl...@gmail.com>.
> On Feb. 4, 2016, 9:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/help.cpp, lines 123-129
> > <https://reviews.apache.org/r/42957/diff/3/?file=1233063#file1233063line123>
> >
> > Is this equivalent to just `return helps.erase(id)`?
Yes, these are equivalent. Updated.
> On Feb. 4, 2016, 9:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/help.cpp, line 107
> > <https://reviews.apache.org/r/42957/diff/3/?file=1233063#file1233063line107>
> >
> > How could the 2nd part of the condition arise? -- below you perform pruning of empty branches.
> >
> > I think either the 2nd part is redundant, or you should perform pruning before returning `false`.
This case can occur if an invalid name is passed in that is not contained in the map. That said, I've updated the logic to be more clear.
- Kevin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/#review117882
-----------------------------------------------------------
On Feb. 4, 2016, 8:30 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2016, 8:30 p.m.)
>
>
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
>
>
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, there was no way to remove an installed help
> string for a given endpoint.
>
> This commit adds the ability to remove these help strings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
> 3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
>
> Diff: https://reviews.apache.org/r/42957/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 42957: Added remove() calls to process::Help.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/#review117882
-----------------------------------------------------------
3rdparty/libprocess/src/help.cpp (line 107)
<https://reviews.apache.org/r/42957/#comment179137>
How could the 2nd part of the condition arise? -- below you perform pruning of empty branches.
I think either the 2nd part is redundant, or you should perform pruning before returning `false`.
3rdparty/libprocess/src/help.cpp (lines 123 - 129)
<https://reviews.apache.org/r/42957/#comment179134>
Is this equivalent to just `return helps.erase(id)`?
- Benjamin Bannier
On Feb. 4, 2016, 9:30 p.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2016, 9:30 p.m.)
>
>
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
>
>
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, there was no way to remove an installed help
> string for a given endpoint.
>
> This commit adds the ability to remove these help strings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
> 3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
>
> Diff: https://reviews.apache.org/r/42957/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 42957: Added remove() calls to process::Help.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/#review117958
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On Feb. 5, 2016, 3:20 a.m., Kevin Klues wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42957/
> -----------------------------------------------------------
>
> (Updated Feb. 5, 2016, 3:20 a.m.)
>
>
> Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
>
>
> Bugs: MESOS-4552
> https://issues.apache.org/jira/browse/MESOS-4552
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Previously, there was no way to remove an installed help
> string for a given endpoint.
>
> This commit adds the ability to remove these help strings.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
> 3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
>
> Diff: https://reviews.apache.org/r/42957/diff/
>
>
> Testing
> -------
>
> Unit test done in a subsequent commit.
>
>
> Thanks,
>
> Kevin Klues
>
>
Re: Review Request 42957: Added remove() calls to process::Help.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/
-----------------------------------------------------------
(Updated Feb. 5, 2016, 3:20 a.m.)
Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
Changes
-------
Addressed comments made by bmahler offline.
Bugs: MESOS-4552
https://issues.apache.org/jira/browse/MESOS-4552
Repository: mesos
Description
-------
Previously, there was no way to remove an installed help
string for a given endpoint.
This commit adds the ability to remove these help strings.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
Diff: https://reviews.apache.org/r/42957/diff/
Testing (updated)
-------
Unit test done in a subsequent commit.
Thanks,
Kevin Klues
Re: Review Request 42957: Added remove() calls to process::Help.
Posted by Kevin Klues <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/42957/
-----------------------------------------------------------
(Updated Feb. 4, 2016, 10:05 p.m.)
Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.
Bugs: MESOS-4552
https://issues.apache.org/jira/browse/MESOS-4552
Repository: mesos
Description
-------
Previously, there was no way to remove an installed help
string for a given endpoint.
This commit adds the ability to remove these help strings.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp 2e76a6a5b1069abce879374a88cea65036873f1d
3rdparty/libprocess/src/help.cpp 2f718b9e160113518fb4a0260db916cb2242dbae
Diff: https://reviews.apache.org/r/42957/diff/
Testing
-------
Thanks,
Kevin Klues