You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2014/10/18 03:58:58 UTC
Review Request 26906: Check "kind" before creating a module instance.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57261
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26903, 26904, 26855, 26906]
All tests passed.
- Mesos ReviewBot
On Oct. 18, 2014, 1:58 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2014, 1:58 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Since, there is no reliable method of converting a template type to a
> string, we create an temporary object of the given kind and compare the
> "kind" fields of the temporary object with that of the one being
> created.
>
>
> Diffs
> -----
>
> src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
> src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57644
-----------------------------------------------------------
src/module/manager.hpp
<https://reviews.apache.org/r/26906/#comment98444>
Why this include?
src/module/manager.hpp
<https://reviews.apache.org/r/26906/#comment98445>
Align according to http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Boolean_Expressions
src/module/manager.hpp
<https://reviews.apache.org/r/26906/#comment98446>
Thanks for the comment! Would it make sense to denote what the 6 NULLs are? (author, version, ...) so if/when more fields are added to the module struct in the future, it is easy to spot/correct this statement too.
src/tests/module_tests.cpp
<https://reviews.apache.org/r/26906/#comment98447>
s/Test/Verify/ ?
- Niklas Nielsen
On Oct. 20, 2014, 6:21 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 6:21 p.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Since, there is no reliable method of converting a template type to a
> string, we create an temporary object of the given kind and compare the
> "kind" fields of the temporary object with that of the one being
> created.
>
>
> Diffs
> -----
>
> src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
> src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57834
-----------------------------------------------------------
Ship it!
Ship It!
- Niklas Nielsen
On Oct. 22, 2014, 10:39 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 10:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduced a new kind<T>() function that returns the string representation of the module "kind". This function is specialized for each module "kind". Also introduced a templatized contains<T>(moduleName) that returns true only if a module of name "moduleName" and kind "T" is available.
>
>
> Diffs
> -----
>
> include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
> src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
> src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
> src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 1:39 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Addressed Nik's comments.
Repository: mesos-git
Description
-------
Introduced a new kind<T>() function that returns the string representation of the module "kind". This function is specialized for each module "kind". Also introduced a templatized contains<T>(moduleName) that returns true only if a module of name "moduleName" and kind "T" is available.
Diffs (updated)
-----
include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 1:13 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Updated description.
Repository: mesos-git
Description (updated)
-------
Introduced a new kind<T>() function that returns the string representation of the module "kind". This function is specialized for each module "kind". Also introduced a templatized contains<T>(moduleName) that returns true only if a module of name "moduleName" and kind "T" is available.
Diffs
-----
include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On Oct. 22, 2014, 12:23 p.m., Niklas Nielsen wrote:
> > src/module/manager.hpp, line 69
> > <https://reviews.apache.org/r/26906/diff/7/?file=728686#file728686line69>
> >
> > Why this change? because of 'kind<Kind>'?
Yeah, it was getting confusing and harder to read.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57808
-----------------------------------------------------------
On Oct. 22, 2014, 4:04 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 4:04 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Since, there is no reliable method of converting a template type to a
> string, we create an temporary object of the given kind and compare the
> "kind" fields of the temporary object with that of the one being
> created.
>
>
> Diffs
> -----
>
> include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
> src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
> src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
> src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Oct. 22, 2014, 9:23 a.m., Niklas Nielsen wrote:
> > src/examples/test_module.hpp, lines 51-52
> > <https://reviews.apache.org/r/26906/diff/7/?file=728685#file728685line51>
> >
> > Why not just return the string directly?
> >
> > return "TestModule"
Ah - never mind. It is because you return a pointer and the string needs to reside somewhere :-/
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57808
-----------------------------------------------------------
On Oct. 22, 2014, 1:04 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 1:04 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Since, there is no reliable method of converting a template type to a
> string, we create an temporary object of the given kind and compare the
> "kind" fields of the temporary object with that of the one being
> created.
>
>
> Diffs
> -----
>
> include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
> src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
> src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
> src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Oct. 22, 2014, 9:23 a.m., Niklas Nielsen wrote:
> > src/examples/test_module.hpp, lines 51-52
> > <https://reviews.apache.org/r/26906/diff/7/?file=728685#file728685line51>
> >
> > Why not just return the string directly?
> >
> > return "TestModule"
>
> Niklas Nielsen wrote:
> Ah - never mind. It is because you return a pointer and the string needs to reside somewhere :-/
Reopening - returning "TestModule" will not give you an address on the stack but a pointer to the string in the data segment. You should be fine returning directly instead of having it backed by a static.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57808
-----------------------------------------------------------
On Oct. 22, 2014, 10:39 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 10:39 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduced a new kind<T>() function that returns the string representation of the module "kind". This function is specialized for each module "kind". Also introduced a templatized contains<T>(moduleName) that returns true only if a module of name "moduleName" and kind "T" is available.
>
>
> Diffs
> -----
>
> include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
> src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
> src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
> src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/#review57808
-----------------------------------------------------------
src/examples/test_module.hpp
<https://reviews.apache.org/r/26906/#comment98677>
Why not just return the string directly?
return "TestModule"
src/module/manager.hpp
<https://reviews.apache.org/r/26906/#comment98679>
Why this change? because of 'kind<Kind>'?
src/module/manager.hpp
<https://reviews.apache.org/r/26906/#comment98678>
Much better! :)
- Niklas Nielsen
On Oct. 22, 2014, 1:04 a.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26906/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 1:04 a.m.)
>
>
> Review request for mesos, Benjamin Hindman and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Since, there is no reliable method of converting a template type to a
> string, we create an temporary object of the given kind and compare the
> "kind" fields of the temporary object with that of the one being
> created.
>
>
> Diffs
> -----
>
> include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
> src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
> src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
> src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
>
> Diff: https://reviews.apache.org/r/26906/diff/
>
>
> Testing
> -------
>
> Added a new test and ran make check.
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 4:04 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs
-----
include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 4:02 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Added mesos::modules::kind<T>() function.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
include/mesos/module.hpp 5205e30c7b8228eced9793aef2348930180811bb
src/examples/test_module.hpp b5cc2fa9147c4fe9a1f2eb3e8c2c2565ea7f7420
src/module/manager.hpp 1c93ac4707e3c5dc8acaaa23861b62d3d3921125
src/tests/module_tests.cpp e92d8717427e259b1fa26b3f17719fd616a748c7
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 5:35 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Addressed Nik's comments.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 9:21 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 9:16 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 1:39 a.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Rebased diff
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya
Re: Review Request 26906: Check "kind" before creating a module
instance.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26906/
-----------------------------------------------------------
(Updated Oct. 18, 2014, 3:09 p.m.)
Review request for mesos, Benjamin Hindman and Niklas Nielsen.
Changes
-------
Added contains<T>(moduleName) and getKind<T>().
Repository: mesos-git
Description
-------
Since, there is no reliable method of converting a template type to a
string, we create an temporary object of the given kind and compare the
"kind" fields of the temporary object with that of the one being
created.
Diffs (updated)
-----
src/module/manager.hpp 56e2af334866d2736a8d43f45daf2dbf5189ce2d
src/tests/module_tests.cpp 54e1e8d26719069d01bfabd918f7c37ddbf3c7c4
Diff: https://reviews.apache.org/r/26906/diff/
Testing
-------
Added a new test and ran make check.
Thanks,
Kapil Arya