mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 63991: Added helpers to create and forward offer operation updates.
Date Fri, 16 Mar 2018 00:59:11 GMT


> On March 14, 2018, 3:56 p.m., Benjamin Bannier wrote:
> > src/common/protobuf_utils.hpp
> > Lines 165-170 (patched)
> > <https://reviews.apache.org/r/63991/diff/4/?file=1974376#file1974376line165>
> >
> >     This looks unusual when compared to other helpers in this file which usually
have arguments modelled pretty closely after the fields of message they produce. The function
names usually also pretty directly reflect what kind of message they produce. This helper
OTOH only can create a single flavor of `Scheduler::Event`.
> >     
> >     Given that this function does not abstract away any logic and will be used just
a single helper added in the next patch, I'd suggest to just inline the creation of `Scheduler::Event`
there for now and not add this helper.

Yea I think you're right, we can just inline this.


- Greg


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


On March 13, 2018, 4:33 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63991/
> -----------------------------------------------------------
> 
> (Updated March 13, 2018, 4:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8190
>     https://issues.apache.org/jira/browse/MESOS-8190
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added helpers to create and forward offer operation updates.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.hpp 2ef1c9a4c2986e6f254e28387635bc01bb0b3259 
>   src/common/protobuf_utils.cpp 9c5fb97afb58f98013b79f3cbaea7dacc3603271 
>   src/master/master.hpp 8bf2c763dafdb7df55c46a56f2ff66f2a951d947 
>   src/master/master.cpp 223ebf29ac4dd1dea9080e4bef4b2d4d064d847f 
> 
> 
> Diff: https://reviews.apache.org/r/63991/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message