You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2015/08/10 20:43:29 UTC
Review Request 37310: Added Appc spec validation utility.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/
-----------------------------------------------------------
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Repository: mesos
Description
-------
Added Appc spec validation utility.
Diffs
-----
src/Makefile.am 942003149b071a322933e2c085d9122903f69713
src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37310/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 13, 2015, 2:39 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/appc/spec.hpp, lines 41-42
> > <https://reviews.apache.org/r/37310/diff/4/?file=1039588#file1039588line41>
> >
> > We wrap comments in 70 char width. Please make sure this is the case:)
I adjusted the line wrapping due to the discussion here: http://www.mail-archive.com/dev@mesos.apache.org/msg33053.html, which I support. What do you think? We should of course make it official on the style guide.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/#review95333
-----------------------------------------------------------
On Aug. 13, 2015, 2:33 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37310/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2015, 2:33 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Appc spec validation utility.
>
>
> Diffs
> -----
>
> src/Makefile.am 942003149b071a322933e2c085d9122903f69713
> src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION
> src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37310/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jie Yu <yu...@gmail.com>.
> On Aug. 13, 2015, 9:39 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/appc/spec.hpp, lines 41-42
> > <https://reviews.apache.org/r/37310/diff/4/?file=1039588#file1039588line41>
> >
> > We wrap comments in 70 char width. Please make sure this is the case:)
>
> Jiang Yan Xu wrote:
> I adjusted the line wrapping due to the discussion here: http://www.mail-archive.com/dev@mesos.apache.org/msg33053.html, which I support. What do you think? We should of course make it official on the style guide.
Then we should at least make it less jagged. This particular comment is very jagged (with only one word in the next line).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/#review95333
-----------------------------------------------------------
On Aug. 13, 2015, 9:33 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37310/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2015, 9:33 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Appc spec validation utility.
>
>
> Diffs
> -----
>
> src/Makefile.am 942003149b071a322933e2c085d9122903f69713
> src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION
> src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37310/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/#review95333
-----------------------------------------------------------
Ship it!
src/slave/containerizer/provisioners/appc/spec.hpp (lines 41 - 42)
<https://reviews.apache.org/r/37310/#comment150246>
We wrap comments in 70 char width. Please make sure this is the case:)
- Jie Yu
On Aug. 13, 2015, 9:33 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37310/
> -----------------------------------------------------------
>
> (Updated Aug. 13, 2015, 9:33 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Appc spec validation utility.
>
>
> Diffs
> -----
>
> src/Makefile.am 942003149b071a322933e2c085d9122903f69713
> src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
> src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION
> src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37310/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/
-----------------------------------------------------------
(Updated Aug. 13, 2015, 3:12 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Changes
-------
Comments. NNFR.
Repository: mesos
Description
-------
Added Appc spec validation utility.
Diffs (updated)
-----
src/Makefile.am 942003149b071a322933e2c085d9122903f69713
src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION
src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37310/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/
-----------------------------------------------------------
(Updated Aug. 13, 2015, 2:33 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Changes
-------
Comments.
Repository: mesos
Description
-------
Added Appc spec validation utility.
Diffs (updated)
-----
src/Makefile.am 942003149b071a322933e2c085d9122903f69713
src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
src/slave/containerizer/provisioners/appc/spec.cpp PRE-CREATION
src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37310/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/#review94986
-----------------------------------------------------------
src/slave/containerizer/provisioners/appc/spec.hpp (line 38)
<https://reviews.apache.org/r/37310/#comment149734>
We usually return Option<Error> for validation functions (see master validation) so that None() means no error and Some() means there's some error.
src/slave/containerizer/provisioners/appc/spec.hpp (line 39)
<https://reviews.apache.org/r/37310/#comment149736>
We try to speed up the compliation. Could you please move the implementation to the cpp file and add some comments about each method in the header?
src/tests/containerizer/appc_provisioner_tests.cpp (line 75)
<https://reviews.apache.org/r/37310/#comment149731>
Add one blank line above.
src/tests/containerizer/appc_provisioner_tests.cpp (line 84)
<https://reviews.apache.org/r/37310/#comment149730>
Add one blank line above.
- Jie Yu
On Aug. 10, 2015, 7:18 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37310/
> -----------------------------------------------------------
>
> (Updated Aug. 10, 2015, 7:18 p.m.)
>
>
> Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added Appc spec validation utility.
>
>
> Diffs
> -----
>
> src/Makefile.am 942003149b071a322933e2c085d9122903f69713
> src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
> src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/37310/diff/
>
>
> Testing
> -------
>
> make check.
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/
-----------------------------------------------------------
(Updated Aug. 10, 2015, 12:18 p.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Changes
-------
Fixed the missing entry in Makefile.am
Repository: mesos
Description
-------
Added Appc spec validation utility.
Diffs (updated)
-----
src/Makefile.am 942003149b071a322933e2c085d9122903f69713
src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37310/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu
Re: Review Request 37310: Added Appc spec validation utility.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37310/
-----------------------------------------------------------
(Updated Aug. 10, 2015, 11:50 a.m.)
Review request for mesos, Chi Zhang, Ian Downes, and Jie Yu.
Changes
-------
Removed a header that's not introduced yet.
Repository: mesos
Description
-------
Added Appc spec validation utility.
Diffs (updated)
-----
src/Makefile.am 942003149b071a322933e2c085d9122903f69713
src/slave/containerizer/provisioners/appc/spec.hpp PRE-CREATION
src/tests/containerizer/appc_provisioner_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/37310/diff/
Testing
-------
make check.
Thanks,
Jiang Yan Xu