Re: Review Request 49223: Enhance value parsing.

2016-07-06 Thread Mesos ReviewBot 

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



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 6, 2016, 2:40 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 6, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-06 Thread Guangya Liu 

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


Ship it!




Ship It!

- Guangya Liu


On 七月 6, 2016, 2:40 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 6, 2016, 2:40 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-05 Thread Klaus Ma 

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

(Updated July 6, 2016, 10:40 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhance value parsing.

2016-07-05 Thread Guangya Liu 


> On 七月 4, 2016, 11:55 a.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.
> 
> Guangya Liu wrote:
> OK, then what about "Check `text resource` in the format of 
> `[a-zA-Z0-9_/.-]`.", ditto for above.
> 
> Klaus Ma wrote:
> 1, Text is not resource; 2. I'd like to depdent on document; otherwise, 
> we need to align TODO and doc.
> 
> Guangya Liu wrote:
> People will be confused by the `text` here and L664 & L666, what about 
> "Check `text value` against document attributes-resources.md."

Check `TEXT value` against document attributes-resources.md.


- Guangya


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


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-05 Thread Guangya Liu 


> On 七月 4, 2016, 11:55 a.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.
> 
> Guangya Liu wrote:
> OK, then what about "Check `text resource` in the format of 
> `[a-zA-Z0-9_/.-]`.", ditto for above.
> 
> Klaus Ma wrote:
> 1, Text is not resource; 2. I'd like to depdent on document; otherwise, 
> we need to align TODO and doc.

People will be confused by the `text` here and L664 & L666, what about "Check 
`text value` against document attributes-resources.md."


- Guangya


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


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-05 Thread Klaus Ma 


> On July 4, 2016, 7:55 p.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.
> 
> Guangya Liu wrote:
> OK, then what about "Check `text resource` in the format of 
> `[a-zA-Z0-9_/.-]`.", ditto for above.

1, Text is not resource; 2. I'd like to depdent on document; otherwise, we need 
to align TODO and doc.


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Guangya Liu 


> On 七月 4, 2016, 11:55 a.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.

OK, then what about "Check `text resource` in the format of 
`[a-zA-Z0-9_/.-]`.", ditto for above.


- Guangya


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


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Mesos ReviewBot 

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



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 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Klaus Ma 


> On July 4, 2016, 7:55 p.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > 
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.

Only [0-9a-z./] is available for Text in document.


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Klaus Ma 

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




src/common/values.cpp (line 665)


Only `[0-9a-z./]` is available for Text in document.


- Klaus Ma


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Guangya Liu 

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




src/common/values.cpp (line 648)


Can you please show more detail for what do you want check here in the 
comment? Do you mean you want to validate the `token` here?



src/common/values.cpp (line 665)


Sorry, I should ask this question in previous patch. Same as above, can you 
please show more comments for what do you want to check here? It would be great 
if you can make the comment easy to understand.


- Guangya Liu


On 七月 4, 2016, 10:33 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 七月 4, 2016, 10:33 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Klaus Ma 


> On July 1, 2016, 1:27 p.m., Guangya Liu wrote:
> > src/tests/values_tests.cpp, line 204
> > 
> >
> > Would it make sense to add some negative case here to test against the 
> > code for error handling?

Will handle in https://issues.apache.org/jira/browse/MESOS-5603 .


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-07-04 Thread Klaus Ma 

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

(Updated July 4, 2016, 6:33 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address Guangya's comments


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


Repository: mesos


Description (updated)
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhance value parsing.

2016-06-30 Thread Guangya Liu 

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




src/common/values.cpp (line 604)


donot update here



src/common/values.cpp (line 620)


I think here should be udpated to

return Error("Expecting one or more 'ranges'");

Using '' is good enough.



src/common/values.cpp (lines 648 - 649)


I think the `[Mesos Attributes & Resources](attributes-resources.md)` will 
not work as a hyperlink here.

s/Text/text

// TODO(klaus1982): Check text against document attributes-resources.md.



src/common/values.cpp (lines 666 - 667)


ditto



src/tests/values_tests.cpp (line 204)


Would it make sense to add some negative case here to test against the code 
for error handling?


- Guangya Liu


On 六月 30, 2016, 5:12 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated 六月 30, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 1. Did not support [1-2, [3-4]] as Ranges; it should be [1-2, 3-4].
> 2. Did not support {a{b, c}d} as Set; it should be {ab, cd}
> 3. Add TODO for checking Text against [a-zA-Z0-9_/.-]
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-06-30 Thread Mesos ReviewBot 

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



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 June 30, 2016, 5:12 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated June 30, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 1. Did not support [1-2, [3-4]] as Ranges; it should be [1-2, 3-4].
> 2. Did not support {a{b, c}d} as Set; it should be {ab, cd}
> 3. Add TODO for checking Text against [a-zA-Z0-9_/.-]
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

2016-06-29 Thread Klaus Ma 

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

(Updated June 30, 2016, 1:12 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address benm's comments; remove checking against Text.


Summary (updated)
-

Enhance value parsing.


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


Repository: mesos


Description (updated)
---

Enhance value parsing.

1. Did not support [1-2, [3-4]] as Ranges; it should be [1-2, 3-4].
2. Did not support {a{b, c}d} as Set; it should be {ab, cd}
3. Add TODO for checking Text against [a-zA-Z0-9_/.-]


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma