You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Charlie Carson <cc...@twitter.com> on 2014/01/14 19:16:06 UTC

Review Request 16839 (was: Review Request 16226: add end point to master for observing health)

including the link to the new review:
https://reviews.apache.org/r/16839/

cc


On Mon, Jan 13, 2014 at 4:42 PM, Charlie Carson <cc...@twitter.com> wrote:

> Hi Ben, I just sent a review that is chopped down to just be changes to
> libprocess.  Basically, it's just adding a post function by refactoring the
> existing get function.  I also gave a more verbose description in both the
> JIRA / CR / git commit.  Finally, I was able to use support/post-reviews.py
> this time (it was giving me errors before), so hopefully that covers it.
>
> thanks and looking forward to round 2 of feedback,
> cc
>
>
> On Tue, Dec 17, 2013 at 3:42 PM, Ben Mahler <be...@gmail.com>wrote:
>
>>    This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/16226/
>>
>> 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 December 12th, 2013, 11:45 p.m. UTC, Charlie Carson wrote:
>>   Review request for mesos, Benjamin Hindman, Jeff Currier, and Vinod
>> Kone.
>> By Charlie Carson.
>>
>> *Updated Dec. 12, 2013, 11:45 p.m.*
>>  *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
>>
>>
>>   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
>>
>>   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)
>>
>> View Diff <https://reviews.apache.org/r/16226/diff/>
>>
>
>