You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by haosdent huang <ha...@gmail.com> on 2015/10/06 06:15:29 UTC
Review Request 39037: Allow description empty in help information.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-3104
https://issues.apache.org/jira/browse/MESOS-3104
Repository: mesos
Description
-------
Allow description empty in help information.
Diffs
-----
3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
Diff: https://reviews.apache.org/r/39037/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 39037: Allow description empty in help information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101677
-----------------------------------------------------------
3rdparty/libprocess/include/process/help.hpp (line 51)
<https://reviews.apache.org/r/39037/#comment159133>
Please use an Option :)
- Ben Mahler
On Oct. 6, 2015, 4:15 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2015, 4:15 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101603
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On 十月 6, 2015, 4:15 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated 十月 6, 2015, 4:15 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by haosdent huang <ha...@gmail.com>.
> On Oct. 8, 2015, 5:12 a.m., Guangya Liu wrote:
> > 3rdparty/libprocess/src/help.cpp, line 61
> > <https://reviews.apache.org/r/39037/diff/2/?file=1092216#file1092216line61>
> >
> > I think that the tldr logic does not needs to be changed, we can keep it in line 47
Mmm, let me revert this change.
- haosdent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101864
-----------------------------------------------------------
On Oct. 7, 2015, 3:04 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 3:04 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101864
-----------------------------------------------------------
3rdparty/libprocess/src/help.cpp (line 52)
<https://reviews.apache.org/r/39037/#comment159366>
I think that the tldr logic does not needs to be changed, we can keep it in line 47
- Guangya Liu
On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated 十月 7, 2015, 3:04 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review102290
-----------------------------------------------------------
3rdparty/libprocess/src/help.cpp (lines 41 - 75)
<https://reviews.apache.org/r/39037/#comment159884>
To make things consistent, how about we take every argument as a const reference here and do the newline append directly to 'help', as you've done for description:
```
string HELP(
const string& tldr,
const Option<std::string>& description,
const Option<string>& references)
{
// Construct the help string.
string help =
"### TL;DR; ###\n" +
tldr;
if (!strings::endsWith(help, "\n")) {
help += "\n";
}
if (description.isSome()) {
help +=
"\n"
"### DESCRIPTION ###\n" +
description.get();
if (!strings::endsWith(help, "\n")) {
help += "\n";
}
}
if (references.isSome()) {
help += "\n";
help += references.get();
}
return help;
}
```
- Ben Mahler
On Oct. 8, 2015, 6:59 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2015, 6:59 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review102291
-----------------------------------------------------------
3rdparty/libprocess/src/help.cpp (line 43)
<https://reviews.apache.org/r/39037/#comment159885>
No need for the std:: qualifier
- Ben Mahler
On Oct. 8, 2015, 6:59 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 8, 2015, 6:59 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review102533
-----------------------------------------------------------
Ship it!
Will get this committed shortly.
3rdparty/libprocess/src/help.cpp (lines 46 - 49)
<https://reviews.apache.org/r/39037/#comment160229>
Let's take tldr as a const string reference as well and just append the newline to 'help' directly, I'll make this cleanup for you.
- Ben Mahler
On Oct. 13, 2015, 2:47 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 2:47 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review102381
-----------------------------------------------------------
Ship it!
Ship It!
- Guangya Liu
On 十月 13, 2015, 2:47 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated 十月 13, 2015, 2:47 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/
-----------------------------------------------------------
(Updated Oct. 13, 2015, 2:47 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Update according to @bmahler's review.
Bugs: MESOS-3104
https://issues.apache.org/jira/browse/MESOS-3104
Repository: mesos
Description
-------
Allow description empty in help information.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
Diff: https://reviews.apache.org/r/39037/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 39037: Allow description empty in help information.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/
-----------------------------------------------------------
(Updated Oct. 8, 2015, 6:59 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Update according @bmahler and @gyliu reviews.
Bugs: MESOS-3104
https://issues.apache.org/jira/browse/MESOS-3104
Repository: mesos
Description
-------
Allow description empty in help information.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
Diff: https://reviews.apache.org/r/39037/diff/
Testing
-------
Thanks,
haosdent huang
Re: Review Request 39037: Allow description empty in help information.
Posted by Guangya Liu <gy...@gmail.com>.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101821
-----------------------------------------------------------
On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated 十月 7, 2015, 3:04 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Guangya Liu <gy...@gmail.com>.
> On 十月 7, 2015, 10:01 p.m., Ben Mahler wrote:
> > 3rdparty/libprocess/src/help.cpp, lines 60-63
> > <https://reviews.apache.org/r/39037/diff/2/?file=1092216#file1092216line60>
> >
> > In your last diff you didn't have this change and the one below to conditionally add newlines, why did you introduce it when changing it to Option?
> >
> > If we want to change this behavior, let's do it in another patch since it seems independent from making 'description' optional. Keep independent changes in separate patches :)
bmahler, I think that the changes are needed to be in this patch since adding "\n" is the old behavior. What haosdent is doing now is updating the logic to align with his new change of making description as optional.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101821
-----------------------------------------------------------
On 十月 7, 2015, 3:04 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated 十月 7, 2015, 3:04 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/#review101821
-----------------------------------------------------------
3rdparty/libprocess/src/help.cpp (lines 51 - 54)
<https://reviews.apache.org/r/39037/#comment159317>
In your last diff you didn't have this change and the one below to conditionally add newlines, why did you introduce it when changing it to Option?
If we want to change this behavior, let's do it in another patch since it seems independent from making 'description' optional. Keep independent changes in separate patches :)
- Ben Mahler
On Oct. 7, 2015, 3:04 a.m., haosdent huang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39037/
> -----------------------------------------------------------
>
> (Updated Oct. 7, 2015, 3:04 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-3104
> https://issues.apache.org/jira/browse/MESOS-3104
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allow description empty in help information.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
> 3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
>
> Diff: https://reviews.apache.org/r/39037/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> haosdent huang
>
>
Re: Review Request 39037: Allow description empty in help information.
Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39037/
-----------------------------------------------------------
(Updated Oct. 7, 2015, 3:04 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
Update according @bmahler's review
Bugs: MESOS-3104
https://issues.apache.org/jira/browse/MESOS-3104
Repository: mesos
Description
-------
Allow description empty in help information.
Diffs (updated)
-----
3rdparty/libprocess/include/process/help.hpp e7dc670648e8abd2fef7be79835f0b71e3e91258
3rdparty/libprocess/src/help.cpp 822c0844c61c7cabfca2b0534a5fb40001bd7cc7
Diff: https://reviews.apache.org/r/39037/diff/
Testing
-------
Thanks,
haosdent huang