> On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.cpp, line 78 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052807#file1052807line78> > > > > Captialize first word (Invalid), same as other error messages below. > > Jojy Varghese wrote: > for internal error messages (that bubbles up), the leaf level messsages > are recommended to start with lower case. This is so that when the root level > logger logs the message, it does not look like : "Error to do operation foo: > Failed to fetch data".
We don't really follow this practice in Mesos, so if you feel like this is something we should do I think you can propose and we can discuss about this. And you never know who is nesting your error message anyways, so you could also end up "Error abc: Error to do op foo: failed to def". For now let's just be consistent with all the code base. > On Aug. 28, 2015, 12:18 a.m., Timothy Chen wrote: > > src/slave/containerizer/provisioners/docker/token_manager.hpp, line 223 > > <https://reviews.apache.org/r/37427/diff/12/?file=1052806#file1052806line223> > > > > Is there a reason you need to include this in the header? Can we just > > put it in cpp? > > Jojy Varghese wrote: > The only reason being that this is a property of the TokenManager. I see, does it need to be? Can't we define a static const in the cpp? - Timothy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review96752 ----------------------------------------------------------- On Aug. 28, 2015, 4:27 a.m., Jojy Varghese wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37427/ > ----------------------------------------------------------- > > (Updated Aug. 28, 2015, 4:27 a.m.) > > > Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. > > > Repository: mesos > > > Description > ------- > > Changes: > - Added Token implementation (RFC 7519). > - Added TokenManager implementation. This component keeps a cache of tokens > requested for any future requests. > > > Diffs > ----- > > src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 > src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION > src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION > src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/37427/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jojy Varghese > >
