Re: Review Request 42957: Added remove() calls to process::Help.

[Ben Mahler]

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

[Kevin Klues]

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

[Kevin Klues]

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

Re: Review Request 42957: Added remove() calls to process::Help.

[Kevin Klues]

> On Feb. 4, 2016, 9:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/help.cpp, lines 123-129
> > 
> >
> > 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
> > 
> >
> > 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.

[Benjamin Bannier]

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




3rdparty/libprocess/src/help.cpp (line 107)


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)


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.

[Kevin Klues]

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

[Kevin Klues]

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

(Updated Jan. 29, 2016, 10:30 p.m.)


Review request for mesos, Ben Mahler, Greg Mann, and Neil Conway.


Changes
---

Reversed order of patches between adding suppport for remove() and exposing 
Help externally with get() functions.


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