One definition of insanity is doing the same thing twice and expecting different results.
I recall that Taras has written basically this same e-mail before. We seem to have this conversation every six months or so. Why do we expect different results this time? If I can propose something that's perhaps different: 1) Write software to figure out who's "slow with reviews". 2) We talk to those people. This is basically the same approach as we had with the tryserver highscore board. It seems to have helped (?). I think Bugzilla archaeology was intended to help with [1], but that site is not useful for this purpose, IMO. But I don't see anything stopping us from actually writing this software. [1] https://metrics.mozilla.com/bugzilla-analysis/ReviewHistory.html On Tue, Jul 9, 2013 at 3:14 PM, Taras Glek <tg...@mozilla.com> wrote: > Hi, > Browsers are a competitive field. We need to move faster. Eliminating review > lag is an obvious step in the right direction. > > I believe good code review is essential for shipping a good browser. > > Conversely, poor code review practices hold us back. I am really frustrated > with how many excellent developers are held back by poor review practices. > IMHO the single worst practice is not communicating with patch author as to > when the patch will get reviewed. > > Anecdotal evidence suggests that we do best at reviews where the patch in > question lines up with reviewer's current project The worst thing that > happens there is rubber-stamping (eg reviewing non-trivial 60KB+ patches in > 30min). > > Anecdotally, latency correlates inversely with how close the reviewer is to > patch author, eg: > > project > same team > same part of organization > org-wide > random > community member > > I think we need change a couple things*: > > a) Realize that reviewing code is more valuable than writing code as it > results in higher overall project activity. If you find you can't write code > anymore due to prioritizing reviews over coding, grow more reviewers. > > b) Communicate better. If you are an active contributor, you should not > leave r? patches sitting in your queue without feedback. "I will review this > next week because I'm (busy reviewing ___ this week|away at conference). I > think bugzilla could use some improvements there. If you think a patch is > lower priority than your other work communicate that. > > c) If you think saying nothing is better than admitting than you wont get to > the patch for a while**, that's passive aggressiveness > (https://en.wikipedia.org/wiki/Passive-aggressive_behavior). This is not a > good way to build a happy coding community. Managers, look for instances of > this on your team. > > In my experience the main cause of review stop-energy is lack of will to > inconvenience own projects by switching gears to go through another person's > work. > > I've seen too many amazing, productive people get discouraged by poor review > throughput. Most of these people would rather not create even more tension > by complaining about this...that's what managers are for :) > > Does anyone disagree with my 3 points above? Can we make some derivative of > these rules into a formal policy(some sort of code of developer conduct)? > > Taras > > * There obvious exceptions to above guidelines (eg deadlines). > ** Holding back bad code is a feature, not a bug, do it politely. > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
