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"
signature.asc
Description: This is a digitally signed message part
