> On 2012-02-29 21:46:16, Benjamin Hindman wrote: > > src/common/attributes.hpp, line 65 > > <https://reviews.apache.org/r/3957/diff/2/?file=75893#file75893line65> > > > > Kill the else and just pull the code out. > > Charles Reiss wrote: > Done. > > Benjamin Hindman wrote: > Really? ;) Since this is so minor I'll tweak it manually before I commit.
Oops, guess I uploaded an earlier version of the patch. > On 2012-02-29 21:46:16, Benjamin Hindman wrote: > > src/common/values.cpp, lines 131-132 > > <https://reviews.apache.org/r/3957/diff/2/?file=75897#file75897line131> > > > > Kill extra space, add '{' at end of line (not sure why that wasn't > > there). > > Charles Reiss wrote: > Done. > > (I assume you're aware that the Google style guide examples use the style > I had, and its cpplint.py checks for two spaces between most // comments and > code.) > > Benjamin Hindman wrote: > No, actually, I was not aware of that. Does it's cpplint.py not like what > we have in our codebase? Yes, e.g.: src/common/values.cpp:131: At least two spaces is best between code and comments [whitespace/comments] [2] src/common/values.cpp:132: At least two spaces is best between code and comments [whitespace/comments] [2] It also, of course, complains about deliberate deviations from the Google styleguide like brace placement of iostream stuff, and gets confused by .hpp instead of .h, as well as some minor things which are probably actually wrong we could make a pass over at some point. - Charles ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3957/#review5468 ----------------------------------------------------------- On 2012-03-01 19:52:41, Charles Reiss wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3957/ > ----------------------------------------------------------- > > (Updated 2012-03-01 19:52:41) > > > Review request for mesos and Benjamin Hindman. > > > Summary > ------- > > I noticed that Attributes were missing an implementation of get() (despite a > prototype) and of operator==. This patch adds them. > > In the process, it does some other minor fixes: > - operator== for Value::* types are moved to common/values.{hpp,cpp} > - tests/attributes_tests.cpp is added as a source of mesos-tests (it was > previously not being compiled!) > > > Diffs > ----- > > src/Makefile.am 5ed41bf > src/common/attributes.hpp 210ceda > src/common/resources.hpp e4a826d > src/common/resources.cpp b036e42 > src/common/values.hpp 902a3d2 > src/common/values.cpp efe6859 > src/tests/attributes_test.cpp 3ceb48f > > Diff: https://reviews.apache.org/r/3957/diff > > > Testing > ------- > > > Thanks, > > Charles > >
