You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2017/11/20 01:02:05 UTC
Review Request 63942: Added a state machine abstraction.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63942/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Repository: mesos
Description
-------
An actor is often implemented like a state machine but the actual
management of the state and transitions is very manual. This
abstraction helps reduce a lot of the boilerplate assocaited with
state machines.
Diffs
-----
3rdparty/libprocess/Makefile.am 03a0ca87f31744c716c99e05aa07242fed480675
3rdparty/libprocess/include/Makefile.am d57a6103881aefef420dc0cf2e85cdf37d560887
3rdparty/libprocess/include/process/state_machine.hpp PRE-CREATION
3rdparty/libprocess/src/tests/CMakeLists.txt b01bf9784563a8f06168f8471222d8da2ffb9b85
3rdparty/libprocess/src/tests/state_machine_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/63942/diff/1/
Testing
-------
make check
Thanks,
Benjamin Hindman
Re: Review Request 63942: Added a state machine abstraction.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63942/#review191468
-----------------------------------------------------------
Fix it, then Ship it!
3rdparty/libprocess/include/process/state_machine.hpp
Lines 166-181 (patched)
<https://reviews.apache.org/r/63942/#comment269299>
It seems a little odd to template these functions instead of pass the state as arguments, ditto elsewhere.
3rdparty/libprocess/src/tests/state_machine_tests.cpp
Lines 21 (patched)
<https://reviews.apache.org/r/63942/#comment269293>
Include for Try?
3rdparty/libprocess/src/tests/state_machine_tests.cpp
Lines 51 (patched)
<https://reviews.apache.org/r/63942/#comment269298>
This looks broken? Should result in a `Try<Try<int>>`?
3rdparty/libprocess/src/tests/state_machine_tests.cpp
Lines 93 (patched)
<https://reviews.apache.org/r/63942/#comment269296>
Do we want to test when the future is notified relative to the completion of the lambda?
- Benjamin Mahler
On Nov. 20, 2017, 1:02 a.m., Benjamin Hindman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63942/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2017, 1:02 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> An actor is often implemented like a state machine but the actual
> management of the state and transitions is very manual. This
> abstraction helps reduce a lot of the boilerplate assocaited with
> state machines.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/Makefile.am 03a0ca87f31744c716c99e05aa07242fed480675
> 3rdparty/libprocess/include/Makefile.am d57a6103881aefef420dc0cf2e85cdf37d560887
> 3rdparty/libprocess/include/process/state_machine.hpp PRE-CREATION
> 3rdparty/libprocess/src/tests/CMakeLists.txt b01bf9784563a8f06168f8471222d8da2ffb9b85
> 3rdparty/libprocess/src/tests/state_machine_tests.cpp PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/63942/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Hindman
>
>