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


Hey Charlie! I haven't taken a look at the full diff just yet, but I wanted to 
make a few comments here in terms of how we do reviews in Mesos.

When a review is eventually pulled down and committed, the review summary and 
description are used in the log message, so we try to keep these sanitary and 
meaningful so that we can directly use these fields as part of the commit 
message / description. This also has the added benefit of helping provide 
context to reviewers.

The second thing is that because libprocess and stout are open source projects 
as well, changes made to these must be done through separate reviews so that we 
can easily contribute changes back to these projects. When changes are mixed 
between mesos / stout / libprocess, this process becomes extremely tedious, 
ideally we would enforce this in the review tooling.

The third thing is that again since we pull down reviews directly as commits, 
we try to separate logical changes into multiple reviews to keep a sane commit 
history for the project (in this case it would be nice to pull out the http 
changes to libprocess, although from my previous comment we would have to do 
this anyway :)). We have a great tool that assists with this process: 
./support/post-reviews.py. This tool takes each commit in your local branch and 
creates / updates a review for each commit. This makes it easy to split changes 
up into multiple commits and it also forces one to keep a meaningful commit 
history. Hope this helps and I look forward to seeing your updates!

- Ben Mahler


On Dec. 12, 2013, 11:45 p.m., Charlie Carson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16226/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2013, 11:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jeff Currier, and Vinod Kone.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-880
>     
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-880
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> add an observe endpoint to master.  add a stubbed RepairCoordinator.
> 
> 
> also reordered the existing routes so they are alphabetical
>  
> add a post method to the libprocess library
> 
> added test / modified master_tests to allow for a MockRepairCoordinator
> ^refactored two methods which were nearly identical into a common path so 
> that I didn't make it triplicate
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 5bdd520c9e0bcc0a2d3a4554cc4ced99dcf78b51 
>   3rdparty/libprocess/src/process.cpp 
> 2d193b13cde92a061b02f903a5d6471ff90cb12a 
>   src/Makefile.am 5f211a244f6f64aef4ababebdb542b40d6086b0b 
>   src/local/local.cpp 83a7f913afb1441e9137c7fcec8fd81815714905 
>   src/master/http.cpp d7cd89f0a3446f4c2e65ecd259544149bf92faf8 
>   src/master/main.cpp b1e45766281c5ea0b8a3cee89e390ea60a97c5e4 
>   src/master/master.hpp 6c168a2cdbd8343516cb47adceaff70c3d46690b 
>   src/master/master.cpp cb8e613b31f39329d61a490f5d3f74fd44ffb08c 
>   src/master/repair_coordinator.hpp PRE-CREATION 
>   src/master/repair_coordinator.cpp PRE-CREATION 
>   src/tests/cluster.hpp 065976c19170e995bd3766bcc7a9b0a244776108 
>   src/tests/master_tests.cpp d34450bf84704b224f4e2dbc61ce100b33d14027 
>   src/tests/mesos.hpp b1239a653e515115a71e436c79b2d11db4a209f9 
>   src/tests/mesos.cpp 5359394f45475803e05d281710139e8cbe7c7364 
> 
> Diff: https://reviews.apache.org/r/16226/diff/
> 
> 
> Testing
> -------
> 
> new tests to verify the end point is passing data to the mock repair 
> coordinator coorectly.
> 
> make check to verify that existing tests are passing
> 
> 
> Thanks,
> 
> Charlie Carson
> 
>

Reply via email to