> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > src/flags/parse.hpp, line 22
> > <https://reviews.apache.org/r/9306/diff/1/?file=255496#file255496line22>
> >
> >     ditto, can you include the 'value' here?

This is the parse function, so this should say why parsing failed, not that it 
failed to parse. Other functions which use this function (see flags/loader.hpp) 
output the value in their error and then concatenate this error. This is the 
model we want to follow for error messages because it composes nicely!


> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > src/flags/parse.hpp, line 43
> > <https://reviews.apache.org/r/9306/diff/1/?file=255496#file255496line43>
> >
> >     ditto, include the value

See above.


> On Feb. 5, 2013, 11:18 p.m., Vinod Kone wrote:
> > third_party/libprocess/include/stout/os.hpp, lines 958-960
> > <https://reviews.apache.org/r/9306/diff/1/?file=255521#file255521line958>
> >
> >     I've seen this code block innumerable times. 
> >     
> >     if (something.isError()) {
> >      return Error(something.error());
> >     }
> >     
> >     Makes me think, we could define and use a macro to perform this. 
> > Thoughts?

I agree, I just don't know how to do this in a type safe way.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9306/#review16145
-----------------------------------------------------------


On Feb. 8, 2013, 5:49 a.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9306/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2013, 5:49 a.m.)
> 
> 
> Review request for mesos, Vinod Kone and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/common/values.cpp 8aebc0b57de110e14c55c5b6d543f17165cfd397 
>   src/detector/detector.cpp ca921151a1fc1a57d265b3439d07a8c45e920f21 
>   src/files/files.cpp 60b567eb62e84ccc99b0b1978733a0ea96560813 
>   src/flags/parse.hpp df7e3480c1f5422d185c86cc5f34e7b87b024a2f 
>   src/linux/cgroups.cpp 03b31e7309b9dd65f00d3b0da2abb81ddaaeea43 
>   src/linux/fs.cpp ea74c3b935dc716fd53827c8a7eb7010961e7aa6 
>   src/linux/proc.cpp 8a0fc48dc9769df35d682ece477246b2df2fc0d4 
>   src/log/coordinator.cpp d9f67bd8b07e6f8b3e022c21f1764d298e314594 
>   src/log/log.hpp 151eebd5d665993dfaac399553a9a315c093762b 
>   src/log/replica.cpp 392cb15991657d8e79637e1c4bc21ec813e3117d 
>   src/logging/logging.cpp e9d778f840f2d6713dbb99d0a8844033be9f0e5f 
>   src/master/frameworks_manager.cpp 189a7e3b4811643baf285ad29f939453257c705f 
>   src/sched/sched.cpp f1eeab6f12ee300d77013c6a4ba62ccd7fdb0d1d 
>   src/slave/cgroups_isolation_module.cpp 
> 63cefc33cf34eebb82db5d8448b751be8652fa36 
>   src/state/leveldb.cpp 6137b5d089dc9ea4b60fd56153b72e14691df3d8 
>   src/state/serializer.hpp 1915cefb102526fbad330b090b8c788bea65ed84 
>   src/state/zookeeper.cpp 4c684797791bb2158d31573d2d47b0da70283bc6 
>   src/tests/master_tests.cpp 948ab5dff34eeba1f3ce593a864ddf282c8b19ed 
>   src/tests/stout_tests.cpp fffef3e42774e9b090a098a219a92e642d303d27 
>   src/zookeeper/group.cpp e74538e3dfa6951178f3e4b49dfd94374a0aa28c 
>   src/zookeeper/url.hpp c62539b30503a94ed82e11fe84d988c122d36bb6 
>   third_party/libprocess/include/process/http.hpp 
> 40248dab3df59b322ad9c5a4858961ef471779fd 
>   third_party/libprocess/include/stout/duration.hpp 
> caf7ce42b34dfb74206d1f10aafabaaec47e7dfc 
>   third_party/libprocess/include/stout/error.hpp PRE-CREATION 
>   third_party/libprocess/include/stout/format.hpp 
> fc349903d31d6e31a8147e3a6be87b37698f28f0 
>   third_party/libprocess/include/stout/fs.hpp 
> 9e62a1b91bc9fac092818ffb3c8bcec46b0bd26d 
>   third_party/libprocess/include/stout/gzip.hpp 
> 6ff1288683a0d3b05098c772945cc2a8b73f8b01 
>   third_party/libprocess/include/stout/net.hpp 
> 1909046017da005a5e7c64d525bb3c741d5ad436 
>   third_party/libprocess/include/stout/numify.hpp 
> 2a59c9269902541d82ba86a2d159984f5bbc6b73 
>   third_party/libprocess/include/stout/os.hpp 
> 712fb209061384b8c045875ef8898a6efd778514 
>   third_party/libprocess/include/stout/protobuf.hpp 
> 25d4634bbc51d14ea78c1a922353f0ce52e458df 
> 
> Diff: https://reviews.apache.org/r/9306/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Reply via email to