On 10/25/2011 03:24 PM, Hugh Brock wrote:
On Tue, Oct 25, 2011 at 02:49:28PM -0400, Matt Wagner wrote:
> Hi all,
>
> I've been spending a little bit of time thinking about the past few
> days, when we frantically reviewed patches and fixed bugs in advance of
> today's demo. Since having a code freeze ahead of the actual deadline is
> something relatively new for us, I don't necessarily see it as a
> tremendous failing that we didn't nail the execution on the first try,
> but I've been thinking about the places I could do better next time. I
> share these musings here hoping to start a constructive discussion.
>
> One thing is that I'd like to get into some sort of regular habit of
> reviewing patches on the mailing list. This sprint, I mostly thought,
> "I'm too busy writing code to bother with reviewing patches," which,
in
> hindsight, was rather selfish to those who DID take time to review
> patches, and to those who needed patches approved. What I think I'd like
> to do is try to get into some sort of rhythm, where I maybe set aside 30
> minutes each day to review patches on the list. It occurs to me that an
> added bonus of reviewing patches is that I get a better sense for what
> other people are doing on the project.
>
> (As long as I'm publicly admitting my faults, I'd also like to stop
> using the morning scrum reports to blindly dump my list of tasks, and
> actually look at what others are working on. That, of course, is what
> I'm meant to be doing anyway, but it's easy to get in the habit of just
> posting my status and moving on.)
>
> I'd also like to propose that we make a little more use of the Doctrine
> of Triviality. (I probably made that term up.) It seems like it's been
> acceptable for a long time to push trivial changes without sending them
> for review -- making a one-line change, reverting a broken commit, and
> so forth. While I am NOT proposing that developers push patches adding
> new features or making large changes on their own, there were a handful
> of patches on the list that were extremely trivial, or that were just
> obviously correct. Some of them sat for days waiting for someone to
> review them. I think it would be okay for people with commit access to
> push these things on their own, but note that they have a very high duty
> of care to make sure they're well-tested. How do other people feel about
> this concept? (For example -- I'd like to add some README files to the
> projects, just a quick overview mostly for the benefit of the GitHub
> pages. I sent one for Conductor some weeks ago that was ignored. I think
> it'd be fine to just push things like that.)
Thanks so much for writing this up, Matt.
I had an interesting conversation with a couple of folks this morning
along these lines. In fact, I'd like to take things a bit farther and
suggest that we could speed things up *a lot* if we removed the
requirement that each patch has to be applied and tested by someone
other than the author. It seems to me it's probably sufficient to ask
patch authors to test their own patches before posting them, and allow
reviewers to sign off after reading the code on-list. I think we could
*at least* leave this to the patch author's discretion -- if they
really want another pair of eyes on a new feature, great, but if
they're comfortable guaranteeing that the patch finishes a task
correctly and that all tests pass, an ACK based on reading the patch
should be sufficient.
What do folks think of this?
Take care,
--Hugh
I think this is worth trying. If we start seeing broken tests on master
again as a result we can always revisit. At the test author's
discretion, as you say, we can still require them -- i.e. "This works
but I haven't been able to verify it on vmware yet -- please test that
before ack" or whatever.
For bugzilla fixes, we may want the tester to verify the fix though.
Scott