ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nikolay Izhikov <nizhi...@apache.org>
Subject Re: Ticket review checklist
Date Thu, 19 Apr 2018 21:53:43 GMT
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
View raw message