> On Feb. 27, 2013, 7:45 p.m., Ben Mahler wrote: > > Unrelated, why did we introduce stout/cache.hpp if it's unused? > > Benjamin Hindman wrote: > It was used for a while before I started leveraging leveldb's cache. Now, > however, stout is it's own thing so there is no such thing as "unused". ;)
Ok, just making sure it's not code that we've never vetted. > 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? 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. > On Feb. 27, 2013, 7:45 p.m., Ben Mahler wrote: > > third_party/libprocess/third_party/stout/README, line 14 > > <https://reviews.apache.org/r/9631/diff/1/?file=262571#file262571line14> > > > > Can include a comment about installation? > > > > I presume that's the only point of configure.ac and Makefile.am. > > > > Are these the steps? > > $ ./bootstrap > > $ ./configure > > $ sudo make install > > Benjamin Hindman wrote: > There is no installation. ;) The only point of configure.ac and > Makefile.am is to enable people to embed stout into their own projects (so > they can use autotools and get make dist for free). And at that point they > should be embedding a release of stout, not a checked out branch from a > repository (thus requiring them to run bootstrap). > > Also, most of the time you don't want people running bootstrap, that's > only for the developers. You can see how we called this out in the Mesos > README. I'll be creating a DEVELOPER file as well to make this even more > explicit in Mesos (and probably do the same in libprocess and stout). 1. Do you mean s/developers/developers of stout itself/, presumably users of stout are developers ;) 2. Ok, that makes sense, but until we _actually_ have stout releases that's not going to be what people do. 3. If I come into the stout github, I'm going to see these configure.ac / bootstrap / Makefile.am files and be confused as to what I'm supposed to use them for. With there being releases available that would go away. The DEVELOPER file would help as well. - Ben ----------------------------------------------------------- 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 > >
