> On Jan. 11, 2016, 1:34 p.m., Alexander Rojas wrote: > > As a patch it looks good, however I feel the reason we created these types > > inheriting from the STL containers was to add a functionality to them (like > > in `hashmap` and `hashset`). But I feel this class doesn't add anything to > > the default `std::list<T>` and indeed it removes functionality since now > > there's no access to all the different constructors from `std::list` and we > > run in the risk of leaking memory (One should not create pointers to any, > > `List`, `hashmap` or `hashset`). > > > > My preferred solution would be to delete this convenience class, and use a > > typedef while the code that uses it is cleaned up, or just remove it > > altogether and cleanup in one pass.
Excellent point, and looking back at the original rr this is what was intended. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41875/#review113743 ----------------------------------------------------------- On Jan. 11, 2016, 2:14 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41875/ > ----------------------------------------------------------- > > (Updated Jan. 11, 2016, 2:14 p.m.) > > > Review request for mesos, Ben Mahler and Till Toenshoff. > > > Bugs: MESOS-4273 > https://issues.apache.org/jira/browse/MESOS-4273 > > > Repository: mesos > > > Description > ------- > > This type was added to provide a variadic constructor around a > std::list. Now that C++11 is used this is not needed anymore as we can > directly invoke std::list's constructor taking a std::initializer_list. > > Review: https://reviews.apache.org/r/41875 > > > Diffs > ----- > > 3rdparty/libprocess/3rdparty/stout/include/Makefile.am > b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe > 3rdparty/libprocess/3rdparty/stout/include/stout/list.hpp > 864d8c9498d66f14ab6fc531965c12fb397b5fe5 > > Diff: https://reviews.apache.org/r/41875/diff/ > > > Testing > ------- > > make check (OS X 10.10.5 and Debian 8) > > > Thanks, > > Benjamin Bannier > >
