mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joris Van Remoortere" <joris.van.remoort...@gmail.com>
Subject Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.
Date Thu, 30 Apr 2015 16:34:42 GMT


> On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote:
> > src/master/framework.cpp, lines 101-102
> > <https://reviews.apache.org/r/33376/diff/3/?file=939881#file939881line101>
> >
> >     The style is either:
> >     Follow the alignment if you stay on the 1st line:
> >     ```
> >     bool Framework::hasExecutor(const SlaveID& slaveId,
> >                                 const ExecutorID& executorId)
> >     ```
> >     or
> >     Indent by 4 if you start the parameter list on a new line:
> >     ```
> >     bool Framework::hasExecutor(
> >         const SlaveID& slaveId,
> >         const ExecutorID& executorId)
> >     ```
> >     
> >     Here and the others below please!
> 
> Marco Massenzio wrote:
>     ummm...
>     // 2: Don't use.
>     allocator->resourcesRecovered(frameworkId, slaveId,
>                                   resources, filters);
>     
>     but I agree I had misread (4):
>     
>     // 4: OK.
>     allocator->resourcesRecovered(
>         frameworkId,
>         slaveId,
>         resources,
>         filters);
>     
>     will change

I think rule 2 might not be clear. It's suggesting that if you align like that, then you should
only have 1 argument per line, not 2.


- Joris


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


On April 22, 2015, 8:35 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33376/
> -----------------------------------------------------------
> 
> (Updated April 22, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2633
>     https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Created new file framework.cpp containing all the methods' implementations for the `Framework`
class (declared in master/master.hpp)
> 
> Declared `operator ==` for Task in type_utils.hpp 
> (it was implemented before, but not declared in the header file);
> 
> Refactored all the LOG(WARNING) to a single utility method.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/Makefile.am 93c7c8a807a33ab639be6289535bbd32022aa85b 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 550d2c50cd01aa5830a7e7e03ab4a0240c221b15 
> 
> Diff: https://reviews.apache.org/r/33376/diff/
> 
> 
> Testing
> -------
> 
> All tests (make check) pass.
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


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