Yep totally understood. Review's can be an FYI, for sure. My general rule of thumb:
1. if code is worked on in an area of the tree that *you* are the only one familiar with, and the change isn't uber significant, go into CTR (commit-then-review) mode, and commit it (that's what version control is for :) ). 2. if code is in an area of code that multiple people are really looking at, and that you want consensus (which is the point of community), then I may throw up a Review Board requesting feedback from the other stewards of those portions of the code, looking for their feedback, etc. In those cases, give them 24-48-72 hours to review, and then get that feedback, and consensus. This is "review then commit" (RTC) mode. 3. if it's a new feature, I may do either CTR or RTC depending on feelings about the social nature of the area of the code/architecture. I generally think CTR is always the way to go, and do so, but will fall back to RTC when I want to gain consensus. HTH. Cheers, Chris ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Chris Mattmann, Ph.D. Senior Computer Scientist NASA Jet Propulsion Laboratory Pasadena, CA 91109 USA Office: 171-266B, Mailstop: 171-246 Email: [email protected] WWW: http://sunset.usc.edu/~mattmann/ ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Adjunct Assistant Professor, Computer Science Department University of Southern California, Los Angeles, CA 90089 USA ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ -----Original Message----- From: Benjamin Hindman <[email protected]> Reply-To: "[email protected]" <[email protected]> Date: Thursday, April 25, 2013 8:16 PM To: mesos <[email protected]> Subject: Re: svn commit: r1471710 - /incubator/mesos/trunk/support/release.sh >Makes sense. Sometimes we throw people on reviews more as an FYI (i.e., >"check this out"). It would be swell if Review Board could distinguish >between the different intentions, but I agree that it's nice to let a >reviewer have time to review. > > >On Thu, Apr 25, 2013 at 8:06 PM, Mattmann, Chris A (398J) < >[email protected]> wrote: > >> Hi Ben, >> >> Just a general note. I had a total of a few hours to look at this >> before you committed it. That really isn't enough time. Typically >> projects give folks at least between 24-72 hours to let folks scope >> something out (or declare otherwise upfront). Apologies I didn't >> get to look at this until now (and I sent in a comment), I've been >> underwater in meetings, etc. >> >> But it would be good in the future to allow others to have a chance >> to take a look. I see you got 2 ship its (1 from vinod, and another >> from benm), which is great, I was on the review too and would have >> liked to scope it too before committing. >> >> No biggie, just wanted to raise this b/c it's a community issue, >> especially for scaling out the project to diverse committers in >> multiple organizations, etc., since they'll need time to review things. >> >> Cheers, >> Chris >> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> Chris Mattmann, Ph.D. >> Senior Computer Scientist >> NASA Jet Propulsion Laboratory Pasadena, CA 91109 USA >> Office: 171-266B, Mailstop: 171-246 >> Email: [email protected] >> WWW: http://sunset.usc.edu/~mattmann/ >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> Adjunct Assistant Professor, Computer Science Department >> University of Southern California, Los Angeles, CA 90089 USA >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> >> >> >> >> >> >> -----Original Message----- >> From: "[email protected]" <[email protected]> >> Reply-To: "[email protected]" >><[email protected] >> > >> Date: Wednesday, April 24, 2013 2:52 PM >> To: "[email protected]" >> <[email protected]> >> Subject: svn commit: r1471710 - >>/incubator/mesos/trunk/support/release.sh >> >> >Author: benh >> >Date: Wed Apr 24 21:52:42 2013 >> >New Revision: 1471710 >> > >> >URL: http://svn.apache.org/r1471710 >> >Log: >> >Fixed bug creating SVN tag in release.sh. >> > >> >Review: https://reviews.apache.org/r/10767 >> > >> >Modified: >> > incubator/mesos/trunk/support/release.sh >> > >> >Modified: incubator/mesos/trunk/support/release.sh >> >URL: >> > >> >>http://svn.apache.org/viewvc/incubator/mesos/trunk/support/release.sh?rev >>= >> >1471710&r1=1471709&r2=1471710&view=diff >> >>>======================================================================== >>>== >> >==== >> >--- incubator/mesos/trunk/support/release.sh (original) >> >+++ incubator/mesos/trunk/support/release.sh Wed Apr 24 21:52:42 2013 >> >@@ -60,7 +60,7 @@ echo "${GREEN}Finally, we'll create an S >> > >> > MESSAGE="Tag for release-${VERSION}-incubating-RC${CANDIDATE}." >> > >> >-git svn branch -n --tag -m ${MESSAGE} \ >> >+git svn branch --tag -m ${MESSAGE} \ >> > release-${VERSION}-incubating-RC${CANDIDATE} || \ >> > { echo "${RED}Failed to create SVN tag/branch${NORMAL}"; exit 1; } >> > >> > >> > >> >>
