> On Feb. 27, 2013, 7:45 p.m., Ben Mahler wrote: > > Makefile.am, line 50 > > <https://reviews.apache.org/r/9631/diff/1/?file=262527#file262527line50> > > > > Sounds like a "NOTE: " to me. > > Benjamin Hindman wrote: > All comments are "notes". I rarely use "NOTE:" unless I'm _REALLY_ trying > to call someones attention to something (and even then I tend to use "N.B."). > What is your distinction? > > Ben Mahler wrote: > I've seen (and been using) NOTE to call attention to something > unintuitive or that requires calling their attention. > > I don't need a NOTE to describe a component or a class, but NOTEs are > useful for me to capture the "gotchas", the unintuitive bits, the parts that > had to break an abstraction, the bits the the original developer had to > _discover_. > > I'll tend to gloss over comments for components or things that I > understand, but the NOTE sections always draw my attention to the details > that are important to be made aware of. That's why I think there's value to > using NOTE comments where appropriate. > > I can see this one not really being a NOTE because it's tied to closely > to the piece of odd code it corresponds to, rather than just a NOTE about > semantics. So I'm fine with this not being a NOTE now that I think about it > more.
I thought about it a bit more and realized that the only time I really use "N.B." is when I have both overview comments and something important. Most comments are typically "something important" and so don't need it, but occasionally there are those comments which have both overview stuff and an important bit. The crux, however, is that usually the important bit requires the context of the overview. Regardless (and alarmingly for me!), I don't see this as a critical component of the coding style. ;) It's more important that we capture things that would require a NOTE directly in the API! - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9631/#review17161 ----------------------------------------------------------- On Feb. 26, 2013, 8:22 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9631/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2013, 8:22 p.m.) > > > Review request for mesos, Vinod Kone and Ben Mahler. > > > Description > ------- > > For now, I decided to eliminate building the stout tests _within_ stout. > Instead, the README explains how to build the stout tests and we do so in > libprocess in order to make sure they actually get run. It probably makes > sense to actually build the tests in stout itself in the future but (a) the > important thing is getting stout independent and (b) the tests still get run > when we build Mesos and (c) it's a header only library anyway so most people > will just want to embed the library and not care about ever having to call > make on it (if they call make on it for anything it will be because it is > being embedded within a distribution as a subdirectory and the outermost > package did a 'make dist'). > > > Diffs > ----- > > Makefile.am 746de0dfda036bf85e04a5fa18711b3ea09e2d3c > src/Makefile.am 8c74525ff6381b6d8624622a758b792fd631cd8c > src/tests/assert.hpp f10f63ae9c2d783fb8ef19ba523eb8144b33d292 > src/tests/fs_tests.cpp 81a10b501c35695989d4070099d422f281d91e89 > src/tests/gc_tests.cpp 411bcc09b28a2543ecd3d083839e10844e8a9939 > src/tests/stout_tests.cpp 6d01330f7009ec94b3b11c89c562eceed4515a3e > src/tests/utils.hpp 4600c2f6e0ceabd6a38f8bb762da314b795c23a0 > third_party/Makefile.am fb8984e8d1bbc527683eec37a1a65cb85fb28fc1 > third_party/libprocess/Makefile.am e5b2ba410744f62d1b2ab47e79a8d0a19eb57370 > third_party/libprocess/configure.ac > 86b938564078702a62a11105b47a45bedd992572 > third_party/libprocess/include/process/gtest.hpp PRE-CREATION > third_party/libprocess/include/stout/cache.hpp > 653507c5f1293acb929d2ffa33738fd830541c84 > third_party/libprocess/include/stout/duration.hpp > 17d2d316fcd994212f41577f54c112e41e061bc6 > third_party/libprocess/include/stout/error.hpp > 97a5cec8988de2cabf158b80a1d34b19ea4faadf > third_party/libprocess/include/stout/exit.hpp > 3da01123eb37d3aa8b0d2c3beead822fdcb19981 > third_party/libprocess/include/stout/fatal.hpp > eabee3ef053727c664ada20841523c4e5c7ffad4 > third_party/libprocess/include/stout/foreach.hpp > 0afe285b310fde673f9ba600c50f81839e9b4c91 > third_party/libprocess/include/stout/format.hpp > 71b5986bc8da1a6a128067a6fc4b48425c9e49e7 > third_party/libprocess/include/stout/fs.hpp > 4dedf5522d62b920b859a0a3098571665496ca5c > third_party/libprocess/include/stout/gzip.hpp > ef36f1bc128d83ce4347aeb38c670ea1c718daba > third_party/libprocess/include/stout/hashmap.hpp > 796cb505fe538d5f65c0f8ea1fa827155f2bb2c1 > third_party/libprocess/include/stout/hashset.hpp > f584545976edaa7d6a80977d5c6006f1a45cb4d0 > third_party/libprocess/include/stout/json.hpp > e2cd4f21cf4a12dbce0748dfa9f51f0014a8c5d3 > third_party/libprocess/include/stout/lambda.hpp > d493353838d95eb2698892a97c5440190a015c45 > third_party/libprocess/include/stout/multihashmap.hpp > a8feabd24dce017ee61f10d3f474ba0db19f2aef > third_party/libprocess/include/stout/net.hpp > 1c5f88aa94dd30a14271809410b7d5f6c9ce2be4 > third_party/libprocess/include/stout/none.hpp > ea8e0f52f11df87d71150f21b318447662e1e986 > third_party/libprocess/include/stout/nothing.hpp > c11a0102dbf646410747f188b100576b340ab71a > third_party/libprocess/include/stout/numify.hpp > d23e23832fca319bb85a831ff56d54fa2f41e422 > third_party/libprocess/include/stout/option.hpp > d19127cd447f26a44ed0da8071b1a8d168c872ca > third_party/libprocess/include/stout/os.hpp > 32638ee273492550491b85223cda8e7a5bde7fa5 > third_party/libprocess/include/stout/owned.hpp > 3433f50f6518a42f5eb3bfedc41a22f0efa24988 > third_party/libprocess/include/stout/path.hpp > fda4e0425aca4e973758c45d005132634c7912e0 > third_party/libprocess/include/stout/protobuf.hpp > eb79e7b0a0cda6ef2086ab03aa2ba71e686cef38 > third_party/libprocess/include/stout/result.hpp > 3326da0042017e76154f4c8f501135e67804c335 > third_party/libprocess/include/stout/stopwatch.hpp > bcff8c63e3988f4b063e16e9bf49a3ef235f19d5 > third_party/libprocess/include/stout/stringify.hpp > 136316d2543396254fbc5659b166e92380be0bca > third_party/libprocess/include/stout/strings.hpp > 47b0d6d454c8ced541910e2ceec8a812556df7dd > third_party/libprocess/include/stout/try.hpp > 787bffd2f619a7a5202b349e0ee70abeca322fab > third_party/libprocess/include/stout/utils.hpp > 0f4bba2ce23a1a03a12d91459525739edd53d150 > third_party/libprocess/include/stout/uuid.hpp > c6c290d2aaa5744ef71af398805ada0a8ba7d14c > third_party/libprocess/third_party/Makefile.am > 150b0d8ad16c5ea3de3a96290d0859bc1a192a15 > third_party/libprocess/third_party/stout/LICENSE PRE-CREATION > third_party/libprocess/third_party/stout/Makefile.am PRE-CREATION > third_party/libprocess/third_party/stout/README PRE-CREATION > third_party/libprocess/third_party/stout/bootstrap PRE-CREATION > third_party/libprocess/third_party/stout/configure.ac PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/cache.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/duration.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/error.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/exit.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/fatal.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/foreach.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/format.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/fs.hpp PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/gtest.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/gzip.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/hashmap.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/hashset.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/json.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/lambda.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/multihashmap.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/net.hpp PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/none.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/nothing.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/numify.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/option.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/os.hpp PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/owned.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/path.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/protobuf.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/result.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/stopwatch.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/stringify.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/strings.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/try.hpp PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/utils.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/include/stout/uuid.hpp > PRE-CREATION > third_party/libprocess/third_party/stout/install-sh PRE-CREATION > third_party/libprocess/third_party/stout/m4/acx_pthread.m4 PRE-CREATION > third_party/libprocess/third_party/stout/tests/tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/9631/diff/ > > > Testing > ------- > > make distcheck > > > Thanks, > > Benjamin Hindman > >
