> On Feb. 15, 2014, 12:08 a.m., Dominic Hamon wrote: > > src/master/http.cpp, line 78 > > <https://reviews.apache.org/r/17976/diff/2/?file=486048#file486048line78> > > > > I can understand avoiding passing non-const references to methods (to > > avoid dangling references) but why would you avoid the non-const reference > > for mutation in this case? > > > > The alternative is more expensive (a roles[role] lookup every time) and > > less readable. > > > > I'll change it if you insist, but I'd like to understand why.
What do you mean by 'every time'? There is a single lookup either way, since each switch case has a 'break'. > On Feb. 15, 2014, 12:08 a.m., Dominic Hamon wrote: > > src/master/http.cpp, line 75 > > <https://reviews.apache.org/r/17976/diff/2/?file=486048#file486048line75> > > > > I prefer to specify that these are JSON::Objects that are being indexed. I don't feel strongly here, it appears that the information about the 'Object' mapping is already apparent from the type (and since this variable is so close to its usage, the concise name seems evident). No? - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17976/#review34554 ----------------------------------------------------------- On Feb. 15, 2014, 12:14 a.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17976/ > ----------------------------------------------------------- > > (Updated Feb. 15, 2014, 12:14 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-692 > https://issues.apache.org/jira/browse/MESOS-692 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > old format of master/state.json: > {... > "slaves":[... > "resources":{"ports":"[31000-32000]"}} > ...] > ...} > > new format: > {... > "slaves":[... > "resources":{"*":{"ports":"[31000-32000]"}}} > ...] > ...} > > There is an assumption that there is each resource has a unique role. If this > is not the case (ie, if there are multiple resources with the role '*' a > warning is emitted. The solution for this is to correctly handle merging of > resources with the same role. > > > Diffs > ----- > > src/master/http.cpp 966eed6d8340038265ef799f1b6149502ccc606e > src/slave/http.cpp c4f598faf6807214608cc89a6d9cf665133f95f3 > src/webui/master/static/js/controllers.js > afb24fb9c2184772f7314162f5637dbabaa2ab94 > > Diff: https://reviews.apache.org/r/17976/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
