> On Jan. 26, 2015, 6:58 p.m., Ben Mahler wrote:
> > include/mesos/resources.hpp, line 247
> > <https://reviews.apache.org/r/30131/diff/2/?file=834317#file834317line247>
> >
> >     Do you want this implicit? Looks like you're being explicit in the .cpp 
> > code when using this.

made it explicit.


> On Jan. 26, 2015, 6:58 p.m., Ben Mahler wrote:
> > include/mesos/resources.hpp, lines 228-233
> > <https://reviews.apache.org/r/30131/diff/2/?file=834317#file834317line228>
> >
> >     It's a bit unusual to define all the child classes in the base class, 
> > the general pattern seems to be:
> >     
> >     ```
> >     T
> >     FooT
> >     BarT
> >     ...
> >     ```
> >     
> >     Any reason to stray from that in this case, but not in other cases?
> >     
> >     Keeping it inside Resources sounds good for now, but it might make it 
> > inconsistent for others to add Filters (since this is a public interface), 
> > maybe add a TODO to pull it out, possibly inside a resources::filters 
> > namespace?

Followed our exiting style: Filter, RoleFilter and PersistentVolumeFilter.


> On Jan. 26, 2015, 6:58 p.m., Ben Mahler wrote:
> > include/mesos/resources.hpp, line 263
> > <https://reviews.apache.org/r/30131/diff/2/?file=834317#file834317line263>
> >
> >     Not yours, but should this be an Option?

Fixed.


- Jie


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


On Jan. 26, 2015, 6:21 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30131/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2015, 6:21 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Used persistent volumes consistently in the code base. (previously, we used 
> persistent disk).
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp 7935e7f9bfe66d1900594dcdcb800c4593a3940f 
>   src/common/resources.cpp 214e441fb86aa0c094c28ed5801089051468137b 
>   src/master/drf_sorter.cpp 967c8aa00606445785836c3e3825a6f467746d33 
>   src/master/master.cpp bda8fda9bc2e52ccc1d75e2541e4604989515e13 
>   src/messages/messages.proto 5bd075e9bfe17be334294b5e4c573eb0233030be 
>   src/slave/containerizer/isolators/posix/disk.cpp 
> bb1779fd644a33527068868a45cf9c19d7732727 
>   src/tests/resource_offers_tests.cpp 
> 4395dd248bd410a2a2887e39a5f9a99d41e2735b 
>   src/tests/resources_tests.cpp b7c1ddfda952f2d6b7f82e90cfe7b6b0aafbc36a 
>   src/tests/sorter_tests.cpp 56e5714c2ab97d0ac81d29e1acb1fbec15471489 
> 
> Diff: https://reviews.apache.org/r/30131/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to