You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2017/06/14 07:44:07 UTC
Review Request 60063: Introduced `roles::isSubroleOf` abstraction.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60063/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-7655
https://issues.apache.org/jira/browse/MESOS-7655
Repository: mesos
Description
-------
See summary.
Diffs
-----
include/mesos/roles.hpp 9a74182bb1462c022e3f06380418383fcd324168
src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6
src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5
Diff: https://reviews.apache.org/r/60063/diff/1/
Testing
-------
Added new tests in `role_tests.cpp` + `make check`
Thanks,
Michael Park
Re: Review Request 60063: Introduced `roles::isSubroleOf` function.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60063/#review178076
-----------------------------------------------------------
Ship it!
Modulo comments.
- Benjamin Mahler
On June 15, 2017, 7:37 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60063/
> -----------------------------------------------------------
>
> (Updated June 15, 2017, 7:37 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7655
> https://issues.apache.org/jira/browse/MESOS-7655
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces a utility to determine whether
> a role is a subrole of another role.
>
>
> Diffs
> -----
>
> include/mesos/roles.hpp 9a74182bb1462c022e3f06380418383fcd324168
> src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6
> src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5
>
>
> Diff: https://reviews.apache.org/r/60063/diff/4/
>
>
> Testing
> -------
>
> Added new tests in `role_tests.cpp` + `make check`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60063: Introduced `roles::isSubroleOf` function.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60063/#review178021
-----------------------------------------------------------
include/mesos/roles.hpp
Lines 38 (patched)
<https://reviews.apache.org/r/60063/#comment251775>
dangling comma?
include/mesos/roles.hpp
Lines 40 (patched)
<https://reviews.apache.org/r/60063/#comment251774>
Hm.. it seems a little surprising that this is a non-strict subrole check. I think I would not expect that, but I suppose you likely needed these semantics in all of your call sites?
Is it possible to instead make this a strict subrole check and have callers perform the extra equality check if they care about that case? E.g.
```
if (isSubroleOf(r1, r2) || r1 == r2)) {
// XXX
}
```
- Benjamin Mahler
On June 15, 2017, 7:37 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60063/
> -----------------------------------------------------------
>
> (Updated June 15, 2017, 7:37 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7655
> https://issues.apache.org/jira/browse/MESOS-7655
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch introduces a utility to determine whether
> a role is a subrole of another role.
>
>
> Diffs
> -----
>
> include/mesos/roles.hpp 9a74182bb1462c022e3f06380418383fcd324168
> src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6
> src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5
>
>
> Diff: https://reviews.apache.org/r/60063/diff/4/
>
>
> Testing
> -------
>
> Added new tests in `role_tests.cpp` + `make check`
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60063: Introduced `roles::isSubroleOf` function.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60063/
-----------------------------------------------------------
(Updated June 15, 2017, 12:37 a.m.)
Review request for mesos and Benjamin Mahler.
Summary (updated)
-----------------
Introduced `roles::isSubroleOf` function.
Bugs: MESOS-7655
https://issues.apache.org/jira/browse/MESOS-7655
Repository: mesos
Description (updated)
-------
This patch introduces a utility to determine whether
a role is a subrole of another role.
Diffs (updated)
-----
include/mesos/roles.hpp 9a74182bb1462c022e3f06380418383fcd324168
src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6
src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5
Diff: https://reviews.apache.org/r/60063/diff/3/
Changes: https://reviews.apache.org/r/60063/diff/2-3/
Testing
-------
Added new tests in `role_tests.cpp` + `make check`
Thanks,
Michael Park