Re: Review Request 49223: Enhance value parsing.

[Benjamin Mahler <bm...@apache.org>]

+dev

Hey Guangya, you gave a ship it on this change, but the summary and
description are the following:

Summary: Enhance value parsing.
Description: Enhance value parsing.

These are pretty vague, if I were to encounter this commit I wouldn't know
what it is doing without reading the code. Keep in mind that giving a "ship
it" means that you're not only happy with the code, but the commit summary
and description are precise and will give context to those that are
scanning the commits.

Also, the ticket for this review (
https://issues.apache.org/jira/browse/MESOS-5739) can be improved.
"Enhance" is a bit of a vague word: it could actually mean increasing the
scope of what we can parse, whereas what this patch is doing is restricting
the scope of what we consider to be valid input.

To me, this ticket and patch are more accurately described as fixing our
Value parsing code to only accept the canonical formats defined in:
http://mesos.apache.org/documentation/latest/attributes-resources/

On Wed, Jul 6, 2016 at 1:46 AM, Mesos ReviewBot <re...@mesos.apache.org>
wrote:

> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
>
> Patch looks great!
>
> Reviews applied: [49223]
>
> Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
>
>
> - Mesos ReviewBot
>
> On July 6th, 2016, 2:40 a.m. UTC, Klaus Ma wrote:
> Review request for mesos and Benjamin Mahler.
> By Klaus Ma.
>
> *Updated July 6, 2016, 2:40 a.m.*
> *Bugs: * MESOS-5739 <https://issues.apache.org/jira/browse/MESOS-5739>
> *Repository: * mesos
> Description
>
> Enhance value parsing.
>
> Testing
>
> make && make check
>
> Diffs
>
>    - src/common/values.cpp (587cb68551d438621e215953e89818b623b7f71b)
>    - src/tests/values_tests.cpp (929861549e3155c33966896f817f9bf9e6d14354)
>
> View Diff <https://reviews.apache.org/r/49223/diff/>
>