mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sirois" <john.sir...@gmail.com>
Subject Re: Review Request: FLAGS (12): More flags integration.
Date Wed, 11 Jul 2012 20:43:19 GMT


> On July 11, 2012, 6:22 p.m., John Sirois wrote:
> > include/mesos/scheduler.hpp, line 43
> > <https://reviews.apache.org/r/5865/diff/1/?file=120852#file120852line43>
> >
> >     you generally call out forward declarations
> 
> Benjamin Hindman wrote:
>     But I do have a comment, above the "previous" forward declaration.

Aha - missed that.


> On July 11, 2012, 6:22 p.m., John Sirois wrote:
> > src/sched/sched.cpp, line 99
> > <https://reviews.apache.org/r/5865/diff/1/?file=120862#file120862line99>
> >
> >     what does this mean?
> 
> Benjamin Hindman wrote:
>     I wasn't sure if I liked having to define the variable as Try<...>::error("uninitialized").
I thought it might be better if a Try is by default in an error "uninitialized" state. Thoughts?

Thats what I thought - I don't like it:

Like Try:error:
  Try::error says what it means and does not seem verbose to me
  Try::error is parallel to Try::some

Don't like default:
  Try having a default constructor that is error is non-obvious - I'd look at use-site code
and then wonder if Try was mutable and the setting would happen later for example.
  Try having a default error constructor _with no message_ is encouraging a bad practice fwict
- make-em at least admit cheeseball by being forced to pass ""


- John


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


On July 9, 2012, 10:27 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5865/
> -----------------------------------------------------------
> 
> (Updated July 9, 2012, 10:27 p.m.)
> 
> 
> Review request for mesos, John Sirois and Vinod Kone.
> 
> 
> Description
> -------
> 
> In particular, integration with slave and scheduler.
> 
> 
> Diffs
> -----
> 
>   include/mesos/scheduler.hpp c307b28 
>   src/Makefile.am 11f6b4c 
>   src/flags/flags.hpp PRE-CREATION 
>   src/local/local.hpp 55f9eaf 
>   src/local/local.cpp d35639f 
>   src/logging/flags.hpp PRE-CREATION 
>   src/master/flags.hpp PRE-CREATION 
>   src/master/main.cpp bcbe35a 
>   src/master/webui.hpp 660d5e1 
>   src/master/webui.cpp 56134b7 
>   src/sched/sched.cpp dc91154 
>   src/slave/flags.hpp PRE-CREATION 
>   src/slave/http.cpp 4a7057a 
>   src/slave/isolation_module.hpp c896908 
>   src/slave/lxc_isolation_module.hpp b7beefe 
>   src/slave/lxc_isolation_module.cpp 2ff1e9c 
>   src/slave/main.cpp c08a55d 
>   src/slave/process_based_isolation_module.hpp f6f9554 
>   src/slave/process_based_isolation_module.cpp 692ad40 
>   src/slave/slave.hpp 80453b0 
>   src/slave/slave.cpp 743e81c 
>   src/slave/webui.hpp cb15ac6 
>   src/slave/webui.cpp 5616542 
>   src/tests/flags_tests.cpp PRE-CREATION 
>   src/tests/slave_tests.cpp 84e8e7d 
>   src/tests/utils.hpp e81ec82 
> 
> Diff: https://reviews.apache.org/r/5865/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message