> On Dec. 17, 2013, 11:42 p.m., Ben Mahler wrote: > > 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!
Thanks for the heads up Ben - (BTW, I was on vacation so sorry for the delay). I'll pull this monolithic review and submit smaller ones scoped to the 3 projects. I'll also scan some of the existing mesos reviews to get a better idea of the correct level of explanation / context for the review comments. thanks again, cc - Charlie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16226/#review30585 ----------------------------------------------------------- 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 > >
