You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@asterixdb.apache.org by Till Westmann <ti...@apache.org> on 2019/06/07 01:07:47 UTC
Code review steps
Hi,
I've proposed an addition to our code review steps on
https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
The only change from the steps we've been doing so far is the use of a
"+1" code review by the author of a change to indicate that the
self-review is done (avoiding the guessing game if a change is awaiting
review).
Please let me know what you think about this approach.
Cheers,
Till
Re: Code review steps
Posted by Taewoo Kim <wa...@gmail.com>.
I forgot whether one can do a second +1 if he/she already did +1. If that
is possible, yeah, your approach is more clear and easy to understand.
Best,
Taewoo
On Sun, Jun 9, 2019 at 7:28 PM Till Westmann <ti...@apache.org> wrote:
> Indeed. That clearly helps.
> However, if there’s more than one round of review we’d need to remove
> reviewers and re-add them.
> A 2nd +1 seems easier (and less error-prone) in that case.
>
> Cheers,
> Till
>
> > On Jun 7, 2019, at 22:04, Taewoo Kim <wa...@gmail.com> wrote:
> >
> > That clearly works. Or why don't we add reviewers only if the self-review
> > is done? :-)
> >
> > Best,
> > Taewoo
> >
> >
> >> On Fri, Jun 7, 2019 at 12:51 AM Mike Carey <dt...@gmail.com> wrote:
> >>
> >> +1 !
> >>
> >>> On 6/6/19 6:07 PM, Till Westmann wrote:
> >>> Hi,
> >>>
> >>> I've proposed an addition to our code review steps on
> >>> https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
> >>>
> >>> The only change from the steps we've been doing so far is the use of a
> >>> "+1" code review by the author of a change to indicate that the
> >>> self-review is done (avoiding the guessing game if a change is
> >>> awaiting review).
> >>>
> >>> Please let me know what you think about this approach.
> >>>
> >>> Cheers,
> >>> Till
> >>
>
>
Re: Code review steps
Posted by Till Westmann <ti...@apache.org>.
Indeed. That clearly helps.
However, if there’s more than one round of review we’d need to remove reviewers and re-add them.
A 2nd +1 seems easier (and less error-prone) in that case.
Cheers,
Till
> On Jun 7, 2019, at 22:04, Taewoo Kim <wa...@gmail.com> wrote:
>
> That clearly works. Or why don't we add reviewers only if the self-review
> is done? :-)
>
> Best,
> Taewoo
>
>
>> On Fri, Jun 7, 2019 at 12:51 AM Mike Carey <dt...@gmail.com> wrote:
>>
>> +1 !
>>
>>> On 6/6/19 6:07 PM, Till Westmann wrote:
>>> Hi,
>>>
>>> I've proposed an addition to our code review steps on
>>> https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
>>>
>>> The only change from the steps we've been doing so far is the use of a
>>> "+1" code review by the author of a change to indicate that the
>>> self-review is done (avoiding the guessing game if a change is
>>> awaiting review).
>>>
>>> Please let me know what you think about this approach.
>>>
>>> Cheers,
>>> Till
>>
Re: Code review steps
Posted by Taewoo Kim <wa...@gmail.com>.
That clearly works. Or why don't we add reviewers only if the self-review
is done? :-)
Best,
Taewoo
On Fri, Jun 7, 2019 at 12:51 AM Mike Carey <dt...@gmail.com> wrote:
> +1 !
>
> On 6/6/19 6:07 PM, Till Westmann wrote:
> > Hi,
> >
> > I've proposed an addition to our code review steps on
> > https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
> >
> > The only change from the steps we've been doing so far is the use of a
> > "+1" code review by the author of a change to indicate that the
> > self-review is done (avoiding the guessing game if a change is
> > awaiting review).
> >
> > Please let me know what you think about this approach.
> >
> > Cheers,
> > Till
>
Re: Code review steps
Posted by Mike Carey <dt...@gmail.com>.
+1 !
On 6/6/19 6:07 PM, Till Westmann wrote:
> Hi,
>
> I've proposed an addition to our code review steps on
> https://cwiki.apache.org/confluence/display/ASTERIXDB/Code+Reviews
>
> The only change from the steps we've been doing so far is the use of a
> "+1" code review by the author of a change to indicate that the
> self-review is done (avoiding the guessing game if a change is
> awaiting review).
>
> Please let me know what you think about this approach.
>
> Cheers,
> Till