asterixdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann" <ti...@apache.org>
Subject Re: [DISCUSS] Improving reviews
Date Tue, 06 Dec 2016 23:36:57 GMT
As I wrote before the proposal was not such much to establish hard rules
but rather to communicate expectations. It seemed to me that hard rules
around this would not be appropriate … but maybe that’s just me.

My initial assumption was that one would start prodding the current
reviewer of choose a different reviewer if things aren’t moving along,
but there probably are a number of options.

Cheers,
Till

On 6 Dec 2016, at 10:06, Steven Jacobs wrote:

> I think we definitely need a max timeout for review responses, but 
> that
> leads immediately to the question: What is the procedure when someone
> doesn't respond in time?
>
> Continuing from Chris, if sufficient time has been given does that 
> imply
> consent?
>
> Steven
>
> On Mon, Dec 5, 2016 at 11:13 PM, Chris Hillery <chillery@hillery.land>
> wrote:
>
>> 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
>> review
>>> [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
>> the
>>> 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
>> on
>>> them
>>> for a while (and we’d 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
>>>
>>

Mime
View raw message