Let's look at the two options: 1. Used Owned<> now only in this location, add a ticket to do a sweep. 2. Use the existing pattern, add a ticket to do a sweep.
Removing a single instance from the sweep you're planning to do doesn't seem to warrant the inconsistency in such a common pattern (Process wrapping). We've already seen inconsistency prove confusing and cause longer reviews for new contributors. I know it's mighty tempting here, but let's try to keep things consistent :) On Fri, Jan 16, 2015 at 12:27 PM, Niklas Nielsen <[email protected]> wrote: > > > > On Jan. 15, 2015, 11:43 a.m., Ben Mahler wrote: > > > This is an extremely common pattern in the code, and so we should > avoid doing it in an inconsistent manner. > > > > > > FWIW, we've yet to see leaks from this pattern, but if you want to > make this change, let's do it as a broad sweep instead of introducing > inconsistent code. :) > > > > Alexander Rukletsov wrote: > > I think this particular example is a canonical case for using > `Owned<>`. I can file a JIRA to sweep the entire codebase, but I would keep > this change. If we agree that using `Owned<>` is preferable here, IMO it's > not worth it to postpone using it for consistency reason, though > consistency is important. > > Did you guys reach consensus? If not, let's jump on a call and/or get some > 3rd party input > > > - Niklas > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29926/#review68303 > ----------------------------------------------------------- > > > On Jan. 15, 2015, 8:58 a.m., Alexander Rukletsov wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/29926/ > > ----------------------------------------------------------- > > > > (Updated Jan. 15, 2015, 8:58 a.m.) > > > > > > Review request for mesos, Bernd Mathiske and Niklas Nielsen. > > > > > > Bugs: MESOS-2213 > > https://issues.apache.org/jira/browse/MESOS-2213 > > > > > > Repository: mesos-git > > > > > > Description > > ------- > > > > See summary. > > > > > > Diffs > > ----- > > > > src/allocation/allocator.hpp PRE-CREATION > > > > Diff: https://reviews.apache.org/r/29926/diff/ > > > > > > Testing > > ------- > > > > make check (Ubuntu, OS X) > > > > > > Thanks, > > > > Alexander Rukletsov > > > > > >
