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 <ccar...@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 <benjamin.mah...@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/> >> > >
