> On Oct. 1, 2014, 7:15 p.m., Ben Mahler wrote: > > Ah much better. > > > > I noticed some of the framework name logging uses single quotes, whereas > > your standard one uses parens. > > The LOG(INFO) wrapping on the next line looks weird to me, but it's pretty > > subjective.
Regarding wrapping, there is no consistency in this file (and the code base) around wrapping log lines :/ I tried to wrap them with some subjective measure of readability (e.g., '"..framework " << *framework' is better than '"..framework" \n *framework') in mind. We can revisit this later if we can figure out some rules. > On Oct. 1, 2014, 7:15 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 3425-3426 > > <https://reviews.apache.org/r/26159/diff/1/?file=708980#file708980line3425> > > > > You don't want logging of the expected pid anymore? forgot to s/frameworkId/*framework/. "*framework" should print the expected pid. > On Oct. 1, 2014, 7:15 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 3933-3936 > > <https://reviews.apache.org/r/26159/diff/1/?file=708980#file708980line3933> > > > > Wonder why the "Slave" line is so short.. and maybe we want to avoid > > the period in favor of a semi-colon here? reformatted to two lines. period seems fine to me like we did on #3905? > On Oct. 1, 2014, 7:15 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 1362-1364 > > <https://reviews.apache.org/r/26159/diff/1/?file=708980#file708980line1362> > > > > Whoops, you indented this one more than the others (4 vs. 2). > > > > Isn't this an extra line vs. > > > > ``` > > LOG(INFO) << "Received registration request for framework" > > << "'" << frameworkInfo.name() << "'" << " at " << from; > > ``` > > > > Seems a bit odd that here you used quotes but in the standard log > > message you used parens to wrap the framework name, should we stick with > > one format? > > > > Ditto elsewhere below. i used quotes for cases when there is no framework id, because it reads better. example: "dropping request for framework 'foobar'" is better than "dropping request for framework (foobar)". - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26159/#review55114 ----------------------------------------------------------- On Sept. 30, 2014, 1:13 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26159/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2014, 1:13 a.m.) > > > Review request for mesos and Ben Mahler. > > > Repository: mesos-git > > > Description > ------- > > Like we did for slave, wanted to standardize how we log about a framework in > master.cpp. Included framework.name() because I think it's more useful for > debugging in a multi-framework world. > > No semantic changes. > > > Diffs > ----- > > src/master/master.hpp d6380199421840aa17d4ce2725dcbcf4a11ce85f > src/master/master.cpp a60308f912a1ed81ecd51c677461a8f591d9eb8e > > Diff: https://reviews.apache.org/r/26159/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
