ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eduard Shangareev <eduard.shangar...@gmail.com>
Subject Re: Ticket review checklist
Date Fri, 20 Apr 2018 12:23:54 GMT
Hi, guys.

I believe that we should update maintainers list before adding this review
requirement.
There should not be the situation when there is only one contributor who is
responsible for a component.
We already have issues with review speed and response time.

On Fri, Apr 20, 2018 at 2:17 PM, Anton Vinogradov <av@apache.org> wrote:

> Vova,
>
> Everything you described sound good to me.
>
> I'd like to propose to create special page at AI Wiki and to describe
> checklist.
> In case we'll find something should be changed/improved it will be easy to
> update the page.
>
> 2018-04-20 0:53 GMT+03:00 Nikolay Izhikov <nizhikov@apache.org>:
>
> > Hello, Vladimir.
> >
> > Thank you for seting up this discussion.
> >
> > As we discussed, I think an important part of this check list is
> > compatibility rules.
> >
> > * What should be backward compatible?
> > * How should we maintain it?
> >
> > > 3) If ticket changes public API or existing behavior, at least two
> > commiters should approve the changes
> >
> > We can learn from other open source project experience.
> > Apache Kafka [1], for example, requires KIP(kafka improvement proposal)
> > for *every* major change.
> > Major change definition includes public API.
> >
> > [1] https://cwiki.apache.org/confluence/display/KAFKA/
> > Kafka+Improvement+Proposals
> >
> >
> > В Чт, 19/04/2018 в 23:00 +0300, Vladimir Ozerov пишет:
> > > Hi Igniters,
> > >
> > > It's glad to see our community becomes larger every day. But as it
> grows
> > it
> > > becomes more and more difficult to manage review and merge processes
> and
> > > keep quality of our decisions at the proper level. More contributors,
> > more
> > > commits, more components interlinked with each other in subtle ways.
> > >
> > > I would like to propose to setup a formal review checklist. This would
> > be a
> > > set of actions every reviewer needs to check before approving merge of
> a
> > > certain feature. Passing the checklist would be *necessary but not
> > > sufficient* phase before commit could be added to the main branch. The
> > > checklist would help us to detect a lot of common problems such a
> broken
> > > tests or bad UX earlier, and would help contributors lead their pull
> > > requests to merge.
> > >
> > > Hallmarks of a good checklist:
> > > - It must be followed be everyone without exceptions
> > > - It must be clear and disallow multiple interpretations
> > > - It must be lightweight, otherwise Ignite development would become a
> > > nightmare
> > > - It must be non-blocking, i.e. inacessibility of a single contributor
> > > should not block ticket progress forever.
> > >
> > > Please let me know if you think the idea makes sense. If we agree on
> it,
> > > let's start defining action items for the checklist. My 2 cents:
> > > 1) All unit tests pass on TC without new failures
> > > 2) If ticket targets specific component, it should be reviewed by
> > > component's maintainer*
> > > 3) If ticket changes public API or existing behavior, at least two
> > > commiters should approve the changes **
> > >
> > > Thoughts?
> > >
> > > Vladimir.
> > >
> > > * TBD: Review component list and define maintainers; define what to do
> if
> > > maintainer is unavailable
> > > ** TBD: Define what is "public API"
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message