[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Code reviews
From: |
Stefan Monnier |
Subject: |
Re: Code reviews |
Date: |
Tue, 08 Mar 2016 11:21:42 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) |
>> That's why I've followed a practice of giving out write access very
>> liberally, with "post-commit spot-check reviews" instead. Indeed, it
> Post-commit reviews also take time.
Indeed. But notice the "spot-check" part: only some of the commits are
actually (post-commit) reviewed.
>> Maybe we could have a half-way system, where commits are pushed to
>> a branch that is "not fast-forward-only", and this branch is then
>> auto-merged to the real (fast-forward-only) master branch after a delay
>> (one day, maybe?) to give time to fix mess ups before they're cast
>> in stone.
> A day is nowhere near enough. IME, a bad commit pushed to master
> takes up to a week to be discovered.
Maybe the delay should depend on the submitter. An infinite delay for
first-submitters and a 0 delay for those submitters we trust to
carefully review they commit messages before pushing (clearly,
I wouldn't be one of them), and all kinds of other values in-between.
> More generally, the problem with such a branch is that it won't be
> much different from pushing to master, except in rare cases that it
> breaks the build, and even that can only be avoided if we set up some
> kind of CI system that continuously builds that branch on the main
> supported platforms.
The review queue could be used for code-quality reviews indeed, but
I was thinking here about focusing on the commit-message reviewing
(since we can fix the code after the fact with additional commits,
whereas we can't fix commit messages after the fact, thanks to Git
implementors's short-sightedness).
Stefan
- Re: Code reviews, (continued)
- Re: Code reviews, Phillip Lord, 2016/03/08
- Re: Code reviews, Yuri Khan, 2016/03/08
- Re: Code reviews, Stefan Monnier, 2016/03/08
- Re: Code reviews, Phillip Lord, 2016/03/09
- Re: Code reviews, Eli Zaretskii, 2016/03/08
- Re: Code reviews, Phillip Lord, 2016/03/09
- Re: Code reviews, Andreas Röhler, 2016/03/10
- Re: Code reviews (was: Is it time to drop ChangeLogs?), Eli Zaretskii, 2016/03/08
- Re: Code reviews,
Stefan Monnier <=
- Re: Code reviews, Eli Zaretskii, 2016/03/08
- Re: Is it time to drop ChangeLogs?, Eli Zaretskii, 2016/03/07
- Re: Is it time to drop ChangeLogs?, Óscar Fuentes, 2016/03/07
- Re: Is it time to drop ChangeLogs?, Eli Zaretskii, 2016/03/08
- Re: Is it time to drop ChangeLogs?, Nikolaus Rath, 2016/03/08
- Re: Is it time to drop ChangeLogs?, Óscar Fuentes, 2016/03/08
- Re: Is it time to drop ChangeLogs?, David Caldwell, 2016/03/08
- Re: Is it time to drop ChangeLogs?, Dmitry Gutov, 2016/03/08
- Re: Is it time to drop ChangeLogs?, John Wiegley, 2016/03/08
- Re: Is it time to drop ChangeLogs?, Dmitry Gutov, 2016/03/08