You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jay Guo <gu...@gmail.com> on 2017/04/20 03:00:19 UTC

Review Request 58552: Resolved a TODO of MULTI_ROLE.

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
-------

After the completion of MULTI_ROLE support, allocation_info is always
populated, some redundant logic for resources of old format are not
necessary anymore, therefore removed.


Diffs
-----

  src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 


Diff: https://reviews.apache.org/r/58552/diff/1/


Testing
-------

make check


Thanks,

Jay Guo


Re: Review Request 58552: Resolved a TODO of MULTI_ROLE.

Posted by Jay Guo <gu...@gmail.com>.

> On June 26, 2017, 2:55 p.m., Benjamin Mahler wrote:
> > src/master/master.hpp
> > Line 2838 (original), 2838-2839 (patched)
> > <https://reviews.apache.org/r/58552/diff/1/?file=1695283#file1695283line2838>
> >
> >     Rather than the comment, we should just call this function allocatedResources so that there's no need for the comment.
> >     
> >     I'll remove the comment in the commit, do you want to send another patch to rename it to `allocatedResources()` for clarity?

https://reviews.apache.org/r/60423/


- Jay


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


On April 20, 2017, 11 a.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58552/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 11 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> After the completion of MULTI_ROLE support, allocation_info is always
> populated, some redundant logic for resources of old format are not
> necessary anymore, therefore removed.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
> 
> 
> Diff: https://reviews.apache.org/r/58552/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


Re: Review Request 58552: Resolved a TODO of MULTI_ROLE.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58552/#review178864
-----------------------------------------------------------


Fix it, then Ship it!





src/master/master.hpp
Line 2838 (original), 2838-2839 (patched)
<https://reviews.apache.org/r/58552/#comment253162>

    Rather than the comment, we should just call this function allocatedResources so that there's no need for the comment.
    
    I'll remove the comment in the commit, do you want to send another patch to rename it to `allocatedResources()` for clarity?


- Benjamin Mahler


On April 20, 2017, 3 a.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58552/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 3 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> After the completion of MULTI_ROLE support, allocation_info is always
> populated, some redundant logic for resources of old format are not
> necessary anymore, therefore removed.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
> 
> 
> Diff: https://reviews.apache.org/r/58552/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>


Re: Review Request 58552: Resolved a TODO of MULTI_ROLE.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58552/#review172449
-----------------------------------------------------------



Patch looks great!

Reviews applied: [58552]

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 April 20, 2017, 3 a.m., Jay Guo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58552/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 3 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> After the completion of MULTI_ROLE support, allocation_info is always
> populated, some redundant logic for resources of old format are not
> necessary anymore, therefore removed.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
> 
> 
> Diff: https://reviews.apache.org/r/58552/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>