Re: Review Request 44777: Added a flags parser for vector to src/common/parse.hpp.

[Ben Mahler]

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


Ship it!





src/common/parse.hpp (line 123)


Do we need this temporary?

```
foreach (const std::string& token, strings::tokenize(value, ",")) {
```



src/common/parse.hpp (line 128)


Unfortunately numify doesn't follow our error composition approach of the 
callee not repeating information available to the caller, so if we followed our 
composition pattern here we would get:

```
return Error("Failed to numify token '" + token + "'"
 ": " + numify.error());

// because numify is repeating caller-available
// information, this yields:
//
// "Failed to numify 'a': Failed to convert 'a' to number"
```

But let's still do this for now, so that this continue to be meaningful 
once we update numify logging to not repeat the arguments available in the 
caller.


- Ben Mahler


On March 18, 2016, 12:14 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44777/
> ---
> 
> (Updated March 18, 2016, 12:14 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4926
> https://issues.apache.org/jira/browse/MESOS-4926
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a flags parser for vector to src/common/parse.hpp.
> 
> 
> Diffs
> -
> 
>   src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 
> 
> Diff: https://reviews.apache.org/r/44777/diff/
> 
> 
> Testing
> ---
> 
> Ran `make` and `make check`
> 
> This is also tested out in a subsequent commit when adding support for the 
> `--nvidia_gpu_devices` flag, which uses this functionality.
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 44777: Added a flags parser for vector to src/common/parse.hpp.

[Kevin Klues]

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

(Updated March 18, 2016, 12:14 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Removed erroneous #include in stout left over from a revision of this patch.


Bugs: MESOS-4926
https://issues.apache.org/jira/browse/MESOS-4926


Repository: mesos


Description
---

Added a flags parser for vector to src/common/parse.hpp.


Diffs (updated)
-

  src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 

Diff: https://reviews.apache.org/r/44777/diff/


Testing
---

Ran `make` and `make check`

This is also tested out in a subsequent commit when adding support for the 
`--nvidia_gpu_devices` flag, which uses this functionality.


Thanks,

Kevin Klues

Re: Review Request 44777: Added a flags parser for vector to src/common/parse.hpp.

[Kevin Klues]

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

(Updated March 15, 2016, 3:11 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Addressed neilc's comments and addressed external comment from bmahler about 
moving this to src/common/parse.hpp.

It is currently not generic enough to live in stout (it is too opinionated 
about the fact that the lists *must* be comma separated, and that (if we 
generalized it) it wouldn't quite work wright for strings containing commas.  
We sould need to come up with some extra rules around formatting for that, 
which we would need to document somehwere.


Summary (updated)
-

Added a flags parser for vector to src/common/parse.hpp.


Bugs: MESOS-4926
https://issues.apache.org/jira/browse/MESOS-4926


Repository: mesos


Description (updated)
---

Added a flags parser for vector to src/common/parse.hpp.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
295eac77d5aba4084bbc044d94f45d660410d725 
  src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 

Diff: https://reviews.apache.org/r/44777/diff/


Testing
---

Ran `make` and `make check`

This is also tested out in a subsequent commit when adding support for the 
`--nvidia_gpu_devices` flag, which uses this functionality.


Thanks,

Kevin Klues