Mailing-List: contact dev-help@asterixdb.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@asterixdb.apache.org
MIME-Version: 1.0
Reply-To: chillery@hillery.land
Sender: ceej@lambda.nu
In-Reply-To: <D14919E0-405E-458A-AE96-425C5490ECF6@apache.org>
References: <D14919E0-405E-458A-AE96-425C5490ECF6@apache.org>
From: Chris Hillery <chillery@hillery.land>
Date: Mon, 5 Dec 2016 23:13:06 -0800
Message-ID: <CAHWyyRw7qCzhuse4nBuEq7Rj+uZ3UK21_N+ADhVu3qGb7+Ezfg@mail.gmail.com>
Subject: Re: [DISCUSS] Improving reviews
To: dev@asterixdb.apache.org
Content-Type: multipart/alternative; boundary=001a11401b0a023f7b0542f824cb
archived-at: Tue, 06 Dec 2016 07:13:35 -0000

--001a11401b0a023f7b0542f824cb
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

It's always been my opinion that code reviews are a very nice-to-have, but
not more than that. The real value in proposing changes for review comes
from the automated testing that can be performed at that stage. I think
we'd be better served overall by shoring up and expanding our automated
testing rather than spending time discussing and implementing non-technical
process.

The main benefits of code reviews are catching large-scale design errors
and spreading code knowledge. You can't really have the former until you
already have the latter - if only one person really understands an area,
nobody else will be able to catch design errors in that area. That's
clearly a risky place to be, but IMHO at least it's not a problem that can
be solved through rules. It requires a cultural shift from the team to make
spreading code knowledge an actual priority, rather than someone everyone
wants but nobody has time or interest to achieve.

If we as a team don't have the drive to do that, then we should accept that
about ourselves and move on. You'll always do best spending time on
enhancing the strengths of a team, not fighting against the things they
don't excel at. I'm also not trying to make any kind of value judgment here
- software development is always about tradeoffs and compromise, risk
versus goals. Any time taken to shift focus towards spreading code
knowledge will by necessity pull from other parts of the development
process, and the upshot may well not be an overall improvement in
functionality or quality.

Ceej
aka Chris Hillery

On Mon, Dec 5, 2016 at 10:49 PM, Till Westmann <tillw@apache.org> wrote:

> Hi,
>
> today a few of us had a discussion about how we could make the reviewing
> process moving along a little smoother. The goal is to increase the
> likeliness
> that the reviews and review comments get addressed reasonably quickly. To
> do
> that, the proposal is to
> a) try to keep ourselves to some time limit up to which a reviewer or
> author
>    responds to a review or a comment and to
> a) regularly report via e-mail about open reviews and how long they have
> been
>    open (Ian already has filed an issue to automate this [1]).
> Of course one is not always able to spend the time to do a thorough revie=
w
> [2]
> / respond fully to comments, but in this case we should aim to let the
> other
> participants know within the time limit that the task is not feasible so
> that
> they adapt their plan accordingly.
> The first proposal for the time limit would be 72h (which is taken from t=
he
> minimal time that a [VOTE] stays open to allow people in all different
> locations and timezones to vote).
> Another goal would be to abandon reviews, if nobody seems to be working o=
n
> them
> for a while (and we=E2=80=99d need to find out what "a while" could be).
>
> Thoughts on this?
> A good idea or too much process?
> Is the time limit reasonable?
> Please let us know what you think (ideally more than a +1 or a -1 ...)
>
> Cheers,
> Till
>
> [1] https://issues.apache.org/jira/browse/ASTERIXDB-1745
> [2] https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
>

--001a11401b0a023f7b0542f824cb--