> On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/attributes.cpp, line 18 > > <https://reviews.apache.org/r/18295/diff/1/?file=498397#file498397line18> > > > > new line. > > > > pull this after 3rd party includes.
the google-style (that we say we follow) is to have the matching header first - do we not do that? > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/attributes.cpp, line 159 > > <https://reviews.apache.org/r/18295/diff/1/?file=498397#file498397line159> > > > > kill this? i find this annotation useful. happy to kill it if you don't. > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/build.cpp, line 19 > > <https://reviews.apache.org/r/18295/diff/1/?file=498399#file498399line19> > > > > pull this after 3rd party includes. see above > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/date_utils.hpp, line 31 > > <https://reviews.apache.org/r/18295/diff/1/?file=498400#file498400line31> > > > > s/date_utils/date/ ? maybe... keeping it with the filename for now but it could be a future cleanup? > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/date_utils.cpp, lines 29-32 > > <https://reviews.apache.org/r/18295/diff/1/?file=498401#file498401line29> > > > > Why do we need to wrap them in a namespace instead of having static > > fields? the anonymous namespace means that they're a) not exposed in the header and b) not available to other translation units to extern. > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/protobuf_utils.cpp, line 1 > > <https://reviews.apache.org/r/18295/diff/1/?file=498404#file498404line1> > > > > this should come after 3rd party includes. see above. > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/common/type_utils.cpp, lines 19-21 > > <https://reviews.apache.org/r/18295/diff/1/?file=498409#file498409line19> > > > > reorder. see above > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/slave/cgroups_isolator.hpp, line 22 > > <https://reviews.apache.org/r/18295/diff/1/?file=498412#file498412line22> > > > > pull this after 3rd party includes. see bove > On Feb. 27, 2014, 11:05 a.m., Vinod Kone wrote: > > src/slave/monitor.cpp, lines 46-56 > > <https://reviews.apache.org/r/18295/diff/1/?file=498423#file498423line46> > > > > Why this optimization? These includes seem too verbose. an attempt to be explicit about which aspects of process are being used. Do you prefer to pull in the whole namespace? - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18295/#review35663 ----------------------------------------------------------- On Feb. 27, 2014, 1:32 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18295/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 1:32 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-1022 > https://issues.apache.org/jira/browse/MESOS-1022 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/executor.hpp 7bc8ecac3f13431e6f7dd45deb06fecf6e0f5d8a > include/mesos/resources.hpp 59f495cdb2a2d55a6c436767789ee579cd5c2f97 > include/mesos/scheduler.hpp 55db1777c4aafd29bdc179653165f4bdcaf5525a > include/mesos/values.hpp 64c138ca6c23ec450e42e3eecf3276e6fc305644 > src/Makefile.am 61d832b89132be2cc5b8ae9bbf743685464f78a4 > src/common/attributes.hpp a08cf188a6d9c27386fd10440d8944f0d7b6fa08 > src/common/attributes.cpp 851c92a88804370313bf052c85f99d483fd67416 > src/common/build.hpp 115e9cc3ec1e28d4622c7884bcbac904b24b95dd > src/common/build.cpp eed08eda4a0db572992187347a2199054ade26d2 > src/common/date_utils.hpp 085c1ce685316b3b96b7564c3613bf775a485054 > src/common/date_utils.cpp 22c7dacb0f5a265fb595cf8cd0486530d8c28d5c > src/common/http.hpp 717bbe99f2ec51a2d1b90730eb19cd54cc82b897 > src/common/lock.cpp 11c8e8c50d806271c36c2ec20633acf06447c37f > src/common/protobuf_utils.hpp 0f653414bc1bb2b632ec8cd9c8bd7202a53d42e1 > src/common/protobuf_utils.cpp PRE-CREATION > src/common/resources.cpp 61c5bda9ed7718e87c405822cf3de4795c51f38f > src/common/thread.hpp 7e487241e419adb6a64109a949ee80a965771b25 > src/common/thread.cpp PRE-CREATION > src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44 > src/common/type_utils.cpp PRE-CREATION > src/common/values.cpp 15583fde45946cfd860d13297a03a927f0523115 > src/master/http.cpp a019f15615d028f3d3628b2611709fc8a3a81bf8 > src/master/master.hpp 72525d2817e52d0eff905571b19fbdffc53228bb > src/master/master.cpp 2e86a1903ce0a0b461eae77078177cc7d28a2659 > src/master/repairer.cpp 151b4ed7ac0b8dacd175b97d372c1f867cd280f6 > src/slave/constants.hpp d237383ff8d00f4731b689a2a1592fdd36f09a4d > src/slave/containerizer/cgroups_launcher.cpp > a9b0108d75d0780d8c6abc82bec859ae4844c0e1 > src/slave/containerizer/isolator.hpp > fc6c9ab10ff535ddd8390aeacf2151ac2d5174a6 > src/slave/containerizer/isolators/posix.hpp > 7fbc6ddd9aa5518870bf938c6ead5eb72d3ec524 > src/slave/containerizer/launcher.cpp > 2361a20f361dc6b360770945329067de95cfd3fc > src/slave/flags.hpp e4d98a53cbfb7f9ca828f17e82d492274cb9969d > src/slave/gc.hpp 7b6fb8365b2e793c019907ecdfbce7413d72f8b4 > src/slave/gc.cpp 3720255704171eaa054c96a790ecd5e6e5304b33 > src/slave/http.cpp 594032da1b2edd47961cd8201acd093b22fa30ae > src/slave/main.cpp a498a6ae6a79c7155c07a5d6dc2d6c9dc8ae060f > src/slave/monitor.hpp c042bc1af31aedf7d2991741d3f8cb70ef353b40 > src/slave/monitor.cpp 1c02986e22bc1dcbc2f07de546bf865d34c41c89 > src/slave/paths.hpp 41bb73d6359ee6122ccc2170512311e1ad4fcc4c > src/slave/paths.cpp PRE-CREATION > src/slave/slave.hpp 01b80dfc1dd55edd6d2f722ff1d4f1bf4d96f28e > src/slave/slave.cpp 4f5349ba75da0bca43c88d33bb663cfa167cbdd3 > src/slave/state.hpp 22f569def3c000856190deff4d55e636afb9e00e > src/slave/state.cpp 21d1fb7fbe695bfc3f80ce17c413261f98e6e0b4 > src/slave/status_update_manager.hpp > 24e3882ad1969c20a64daf90e408618c310e9a81 > src/slave/status_update_manager.cpp > 9db53e8b2a6440b7eebe3bc61912b170bde7a473 > src/tests/slave_recovery_tests.cpp 40a9599787918b78790462e81729ec7ac2395509 > src/webui/master/static/js/controllers.js > afb24fb9c2184772f7314162f5637dbabaa2ab94 > > Diff: https://reviews.apache.org/r/18295/diff/ > > > Testing > ------- > > No functional change. Ran clean build and make check. > > Timed build: > before change - 26m30s > after change - 19m33s > > diff stats: 44 files changed, 1067 insertions(+), 1074 deletions(-) > > > Thanks, > > Dominic Hamon > >
