mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 68141: Moved Framework implementation into separate file.
Date Thu, 02 Aug 2018 12:18:14 GMT


> On Aug. 2, 2018, 10:33 a.m., Alexander Rukletsov wrote:
> > src/master/master.hpp
> > Lines 2981-2983 (original)
> > <https://reviews.apache.org/r/68141/diff/2/?file=2066246#file2066246line2981>
> >
> >     I think these qualify to stay in ".hpp".

Letting functions stay in header files can of course be good for inlining, but there are advantages
to moving even "small" functions to separate object files:
- it makes the function easier to grep for since one can use `Framework::active` as pattern
- it makes it easier to find the function when it is defined in the same file as all the other
functions
- it removes subjectivity about when a function is "simple enough" to be defined inline from
subsequent reviews

I moved them back to the `.hpp` in the subsequent review, but I changed them to be out-of-class
definitions due to reasons (1) and (3) above.


- Benno


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


On Aug. 2, 2018, 12:17 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68141/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2018, 12:17 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-2633
>     https://issues.apache.org/jira/browse/MESOS-2633
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change aims to reduce compile time and cognitive
> load when browsing the "master.hpp" header.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 398ffdddb757e96cfeda376bd983823b9a93904e 
>   src/Makefile.am 2ad719e408e56294d308dca1da9f6ea800450a66 
>   src/master/framework.cpp PRE-CREATION 
>   src/master/master.hpp 45ffedbc314941f44a5330e78cf7dd2e402c9116 
>   src/master/master.cpp 192fe8299777f68bf6f48d693ef79ff75fecfe00 
> 
> 
> Diff: https://reviews.apache.org/r/68141/diff/3/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


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