+1 on 72h- seems reasonable. The forthcoming reporting can clearly indicate the age of reviews and if they are approaching / exceeding the SLA.
I think it would be good to get code coverage information integrated with the reviews- it seems helpful to know if added / modified code is being exercised by tests. On Wed, Dec 7, 2016 at 12:25 PM Ian Maxon <[email protected]> wrote: > I think that if we can all agree on what a reasonable upper-bound timeframe > is for addressing reviews and comments is, it'd help a lot. 72h seems fine > to me. I think that we all have been on either side of having a review held > up and it can get aggravating and draw things out un-necessarily. > > On Wed, Dec 7, 2016 at 7:31 AM, Murtadha Hubail <[email protected]> > wrote: > > > I’m for any type of improvement of the current code review process. > > Currently, I have only few hours a week to contribute to the project and > > almost every week I start working on a new change. As the number of > touched > > classes on the change increases, I just decide to abandon it and start > > looking for another change that might be smaller. I do this because I > know > > the bigger change will be stuck in the code review queue and when that > > happens I feel discouraged from contributing and my contribution doesn’t > > add a value to the system. > > > > Even though the proposed idea might not solve all the issues in the > > current code review process, I believe it is definitely going to improve > it > > (and encourage more contributions). > > > > Cheers, > > Murtadha > > > On Dec 6, 2016, at 9:49 AM, Till Westmann <[email protected]> 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 > > > > >
