> On Oct. 24, 2018, 5:22 p.m., Benjamin Mahler wrote: > > src/tests/allocator.hpp > > Lines 40-45 (patched) > > <https://reviews.apache.org/r/69096/diff/1/?file=2101109#file2101109line40> > > > > I think we some other create helpers lying around, e.g. createTask. Is > > this where these belong? > > > > Do you think we need them? > > Meng Zhu wrote: > I do not have use cases of other helpers in mind atm. Thus I prefer to > pull in only these two. > > Benjamin Mahler wrote: > Do we even need these helpers? They seem pretty trivial, and I wonder how > much code it's actually simplifying. I'm fine with the patch, but it feels > heavy to have a new header and compliation unit just for these, especially > since they're only used in two files and are trivial.
Yeah, we may do these two inline or just copy them around. But I expect that the allocator tests and benchmarks would share more utilities in the future (after all, they are all allocator tests). So might as well set up the room for it. If it makes the code structure cleaner, I think it is worth it. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69096/#review210010 ----------------------------------------------------------- On Oct. 19, 2018, 6:28 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69096/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2018, 6:28 p.m.) > > > Review request for mesos, Benjamin Mahler and Gastón Kleiman. > > > Repository: mesos > > > Description > ------- > > This helps to share the helpers between > `hierarchical_allocator_tests.cpp` and > `hierarchical_allocator_benchmarks.cpp`. > > > Diffs > ----- > > src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 > src/tests/CMakeLists.txt 00dbdee1c06e7571fa799850cdaf7f2ccadc8bea > src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b > src/tests/allocator.cpp PRE-CREATION > src/tests/hierarchical_allocator_tests.cpp > 27fbd9cf0c4442e7675362a806d35bad141ffb6d > > > Diff: https://reviews.apache.org/r/69096/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
