mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 63852: Added "Task" prefix to status update manager related classes/methods.
Date Thu, 16 Nov 2017 18:52:10 GMT

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




src/local/local.cpp
Line 102 (original), 102 (patched)
<https://reviews.apache.org/r/63852/#comment268888>

    `grep StatusUpdateManager` yields a couple other sites which we might want to update:
    
    https://github.com/apache/mesos/blob/c0a47b696291e152c0a4d772b2b38ab2aad8c0bb/src/messages/messages.proto#L72
    https://github.com/apache/mesos/blob/c0a47b696291e152c0a4d772b2b38ab2aad8c0bb/src/tests/slave_recovery_tests.cpp#L356
    https://github.com/apache/mesos/blob/c0a47b696291e152c0a4d772b2b38ab2aad8c0bb/src/tests/slave_recovery_tests.cpp#L4605
    
    `grep "status update manager"` also yields a number of comments which could be updated,
if desired; I'll leave that up to you.



src/slave/status_update_manager.cpp
Line 285 (original), 288 (patched)
<https://reviews.apache.org/r/63852/#comment268864>

    Update this logging, as elsewhere?



src/slave/status_update_manager.cpp
Line 512 (original), 517 (patched)
<https://reviews.apache.org/r/63852/#comment268867>

    You can leave these two parameters on two separate lines, it's more consistent with our
style elsewhere.



src/tests/cluster.cpp
Line 506 (original), 506 (patched)
<https://reviews.apache.org/r/63852/#comment268878>

    s/status/task status/ ?


- Greg Mann


On Nov. 15, 2017, 11:41 p.m., Gaston Kleiman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63852/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8197
>     https://issues.apache.org/jira/browse/MESOS-8197
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> We're about to add an offer operation status update manager and a
> generic `StatusUpdateManagerProcess` class, which would conflict with
> the name of the current classes/methods/variables.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 99c1b087d75a1759ed61a4e6178e15919bee1e77 
>   src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
>   src/slave/slave.hpp 40442f25ae1b223380e11f8602e6256c9203ef47 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/slave/status_update_manager.hpp 14ead42f22e342d03c117eabe7af635b48052703 
>   src/slave/status_update_manager.cpp e0d029b855de1024882a0db3c2556523240179e5 
>   src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
>   src/tests/cluster.cpp d657da6d84e5ea28e0dad180dce069de569c5d38 
>   src/tests/mock_slave.hpp 57189cee20511d145ae6b47a4dc2c66a14638dad 
>   src/tests/mock_slave.cpp db24f9e5b71f558d2f811f0da8a9cc9c7c2dd341 
>   src/tests/status_update_manager_tests.cpp 24d6a9951b164369b2a5875e1c54b7e69e22d66b

> 
> 
> Diff: https://reviews.apache.org/r/63852/diff/1/
> 
> 
> Testing
> -------
> 
> Tests still pass on GNU/Linux.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>


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