> On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 192 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line192> > > > > touch doesn't truncate - this probably should not either ... or else > > the name should change.
agreed. s/touch/create > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 208 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line208> > > > > You copied local style as in the inlined open method above, but I > > suspect that was itself a typo based on other inline functions in this file > > and other files in mesos. -2 indent here and elsewhere on function opening > > { and closing } ? yea..stupid eclipse auto formatting. fixed > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 222 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line222> > > > > You own this TODO actually, i just copied this from #707 > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 255 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line255> > > > > SEEK_SET or should the doc be ammended to say read the file from its > > current offset to the end? amended the doc > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 263 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line263> > > > > This could error as well as the 2 lseeks above, seems like that should > > be handled and propagated via the Try return type. fixed > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 521 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line521> > > > > You should s/pattern/prefix/ - afaict thats all you will successfully > > match. its not just the prefix, it could match anywhere in the file name. > On 2012-05-03 22:28:33, John Sirois wrote: > > src/common/utils.hpp, line 545 > > <https://reviews.apache.org/r/4899/diff/4/?file=106486#file106486line545> > > > > Instead of a recursive function, why not leverage ftw and even fnmatch > > if you want some globbiness. is your concern that the stack might get too deep or something else? talked to ben offline about using ftw. since ftw uses function pointers, we would need to move the find() to utils.cpp. we are trying to get rid of utils.cpp to make it easier for third-parties to use our utils! added a TODO for now. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4899/#review7518 ----------------------------------------------------------- On 2012-05-03 19:23:34, Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4899/ > ----------------------------------------------------------- > > (Updated 2012-05-03 19:23:34) > > > Review request for mesos and John Sirois. > > > Summary > ------- > > We can now read/write strings to a file. > > Some common utils for working with protobuf objects. > > > Diffs > ----- > > src/Makefile.am cd503a8 > src/common/protobuf_utils.hpp PRE-CREATION > src/common/utils.hpp 1d81e21 > src/slave/slave.cpp b233b68 > src/tests/protobuf_io_tests.cpp 22f37ac > src/tests/utils_tests.cpp 49ec107 > > Diff: https://reviews.apache.org/r/4899/diff > > > Testing > ------- > > make check > > > Thanks, > > Vinod > >
