On Wed, Jun 1, 2011 at 1:13 AM, Brion Vibber <br...@pobox.com> wrote:
> "OMG my commit will be wasted in 20 >> minutes! Heeelponeone" is not the best scenario for clear thinking. >> Time pressure might result in people reviewing stuff they're less >> familiar with, and reviews being done in a haste, so CR quality will >> suffer. > > > I'd say better to do that continuously and confirm that things are being > reviewed *and* run on automated tests *and* run against humans on > actual-production-use beta servers *and* in actual production use within a > reasonable time frame, than to let review slide for 10 months and rush it > all right before a giant release. > > That IMO is what really hurts code review -- removing it from the context > of code *creation*, and removing the iterative loop between writing, > debugging, testing, debugging, deploying, debugging, fixing, and debugging > code. > I would wholeheartedly supoport the idea of making tests a mandatory accompaniment to code. One latest example: we made latest security releases without tests. This is bad not only because bugs not tested for *will* eventually reappear, but also because in my experience the process of writing tests helps to discover more problems with code. If compared with manual tests, writing low-level unit tests allows to look at possible problems more methodicallly, uncovering more problems. Ideally, I would like to see the same policy as Chome has: 1) All reasonably testable code must be accompanied with automatic tests, period. (It's easier to enforce for them because they practice pre-commit reviews.) 2) To commit, hang around on IRC. After committing, wait for buildbot to post test results of your commit. If something's broken, fix it ASAP, otherwise you'll be reverted. To make this reasonable, we would also need to run tests for branches on the central server, too - CruiseControl doesn't look flexible enough for this. _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
