> On Dec. 19, 2014, 4:48 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 3922-3923 > > <https://reviews.apache.org/r/27760/diff/9/?file=797824#file797824line3922> > > > > I'd hold off on adding these @params, since they don't belong in .cpp > > files IIUC, and we don't use them generally. This looks like poor naming of > > arguments IMO, not sure if you saw my comment on Adam's review? > > Till Toenshoff wrote: > I did and tried to answer but forgot to publish. :( > > However, now that you mention @param is a header only concept (which is > AFAIK not enforced but could be our policy), then yes I have to entirely > agree with you...
It certainly makes sense to put @params in headers (although I do see @params throughout src/linux/cgroups.cpp), since their purpose is describing the function as an API. However, in this case, we need the clarification for the sake of readability within the function, rather than descriptiveness to the caller, so I would vote for non-@param comments here in the cpp, or more descriptive variable names (if our protobuf handler standards didn't prevent it). Till, I'll wait for another quick iteration before committing anything. - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review65711 ----------------------------------------------------------- On Dec. 19, 2014, 4:41 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2014, 4:41 p.m.) > > > Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-2050 > https://issues.apache.org/jira/browse/MESOS-2050 > > > Repository: mesos-git > > > Description > ------- > > The initial design and implementation of the authenticator module interface > caused issues and was not optimal for heavy lifting setup of external > dependencies. By introducing a two fold design, this has been decoupled from > the authentication message processing. The new design also gets us back on > track to the goal of makeing SASL a soft dependency of mesos. > > > Diffs > ----- > > src/authentication/authenticator.hpp 460494a > src/authentication/cram_md5/authenticator.hpp d739a02 > src/master/master.hpp 26116af > src/master/master.cpp 0c98b51 > src/tests/cram_md5_authentication_tests.cpp a356aa1 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
