Vova, Looks good to me.
Please add clear explanation how to check 1.4, 1.5 and 2.x. Also, this should be published as a wiki page with refs to... eg. Coding Guidelines. пн, 7 мая 2018 г. в 17:26, Vladimir Ozerov <voze...@gridgain.com>: > Igniters, > > This is the checklist I have at the moment. Please let me know if you have > any comments on existing items, or want to add or remove anything. It looks > like we may have not only strict rules, but *nice to have* points here as > well with help of *MUST*, *SHOULD* and *MAY* words as per RFC2119 [1]. So > please feel free to suggest optional items as well. > > 1) API > 1.1) API compatibility *MUST* be maintained between minor releases. Do not > remove existing methods or change their signatures, deprecate them instead > 1.2) Default behaviour "SHOULD NOT* be changed between minor releases, > unless absolutely needed. If change is made, it *MUST* be described in > "Migration Guide" > 1.3) New operation *MUST* be well-documented in code (javadoc, dotnetdoc): > documentation must contain method's purpose, description of parameters and > how their values affect the outcome, description of return value and it's > default, behavior in negative cases, interaction with other operations and > components > 1.4) API parity between Java and .NET platforms *SHOULD* be maintained when > operation makes sense on both platforms. If method cannot be implemented in > a platform immediately, new JIRA ticket *MUST* be created and linked to > current ticket > 1.5) API parity between thin clients (Java, .NET) *SHOULD* be maintained > when operation makes sense on several clients. If method cannot be > implemented in a client immediately, new JIRA ticket *MUST* be created and > linked to current ticket > 1.6) All exceptions thrown to a user *MUST* have explanation how to > resolve, workaround or debug an error > > 2) Compatibility > 2.1) Persistence backward compatibility *MUST* be maintained between minor > releases. It should be possible to start newer version on data files > created by the previous version > 2.2) Thin client forward and backward compatibility *SHOULD* be maintained > between two consecutive minor releases. If compatibility cannot be > maintained it *MUST* be described in "Migration Guide" > 2.3) JDBC and ODBC forward and backward compatibility *SHOULD* be > maintained between two consecutive minor releases. If compatibility cannot > be maintained it *MUST* be described in "Migration Guide" > > 3) Tests > 3.1) New functionality *MUST* be covered with unit tests for both positive > and negative use cases > 3.2) All test suites *MUST* be run before merge to master..There *MUST* be > no new test failures > > 4) Code style *MUST* be followed as per Ignite's Coding Guidelines > > Vladimir. > > [1] https://www.ietf.org/rfc/rfc2119.txt > > On Fri, May 4, 2018 at 4:33 PM, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > Hi Dmitry, > > > > Yes, I'll do that in the nearest days. > > > > On Wed, Apr 25, 2018 at 8:24 PM, Dmitry Pavlov <dpavlov....@gmail.com> > > wrote: > > > >> Igniters, the idea was related to small refactorings co-located with > main > >> change. > >> > >> Main change itself indicates that existing code did not meet the > criteria > >> of practice. Approving of standalone refactorings instead contradicts > with > >> principle don't touch if it works. So I still like idea of co-located > >> changes improving code, javadocs, style, etc. > >> > >> But let's not argue about this point now, let's summarize the undisputed > >> points and add it to the wiki. Vladimir, would you please do it? > >> > >> > >> ср, 25 апр. 2018 г. в 16:42, Nikolay Izhikov <nizhi...@apache.org>: > >> > >> > Igniters, > >> > > >> > I agree with Vova. > >> > > >> > Don't fix if it works! > >> > > >> > If you 100% sure then it a useful addition to the product - just make > a > >> > separate ticket. > >> > > >> > В Ср, 25/04/2018 в 11:44 +0300, Vladimir Ozerov пишет: > >> > > Guys, > >> > > > >> > > The problem with in-place refactorings is that you increase affected > >> > scope. > >> > > It is not uncommon to break compatibility or public contracts with > >> even > >> > > minor things. E.g. recently we decided drop org.jsr166 package in > >> favor > >> > of > >> > > Java 8 classes. Innocent change. Result - broken storage. Another > >> problem > >> > > is conflicts. It is not uncommon to have long-lived branches which > we > >> > need > >> > > to merge with master over and over again. And a lot of refactorings > >> cause > >> > > conflicts. It is much easier to resolve them if you know that logic > >> was > >> > not > >> > > affected as opposed to cases when you need to resolve both renames > and > >> > > method extractions along with business-logic changes. > >> > > > >> > > I'd like to repeat - if you have a time for refactoring then you > >> > definitely > >> > > have a time to extract these changes to separate PR and submit a > >> separate > >> > > ticket. I am quite understand what "low priority" do you mean if you > >> do > >> > > refactorings on your own. > >> > > > >> > > On Tue, Apr 24, 2018 at 10:52 PM, Andrey Kuznetsov < > stku...@gmail.com > >> > > >> > > wrote: > >> > > > >> > > > +1. > >> > > > > >> > > > Once again, I beg for "small refactoring permission" in a > checklist. > >> > As of > >> > > > today, separate tickets for small refactorings has lowest > priority, > >> > since > >> > > > they neither fix any flaw nor add new functionality. Also, the > >> > attempts to > >> > > > make issue-related code safer / cleaner / more readable in "real" > >> pull > >> > > > requests are typically rejected, since they contradict our current > >> > > > guidelines. > >> > > > > >> > > > I understand this will require a bit more effort from > >> > committer/maintainer, > >> > > > but otherwise we will get constantly degrading code quality. > >> > > > > >> > > > > >> > > > 2018-04-24 18:52 GMT+03:00 Eduard Shangareev < > >> > eduard.shangar...@gmail.com > >> > > > > : > >> > > > > Vladimir, > >> > > > > > >> > > > > I am not talking about massive/sophisticated refactoring. But I > >> > believe > >> > > > > that ask to extract some methods should be OK to do without an > >> extra > >> > > > > ticket. > >> > > > > > >> > > > > A checklist shouldn't be necessarily a set of certain rules but > >> also > >> > it > >> > > > > could include suggestion and reminders. > >> > > > > > >> > > > > On Tue, Apr 24, 2018 at 6:39 PM, Vladimir Ozerov < > >> > voze...@gridgain.com> > >> > > > > wrote: > >> > > > > > >> > > > > > Ed, > >> > > > > > > >> > > > > > Refactoring is a separate task. If you would like to rework > >> > exchange > >> > > > > > >> > > > > future > >> > > > > > - please do this in a ticket "Refactor exchange task", nobody > >> would > >> > > > > > >> > > > > against > >> > > > > > this. This is just a matter of creating separate ticket and > >> > separate > >> > > > > >> > > > PR. > >> > > > > If > >> > > > > > one have a time for refactoring, it should not be a problem > for > >> > him to > >> > > > > > spend several minutes on JIRA and GitHub. > >> > > > > > > >> > > > > > As far as documentation - what you describe is normal review > >> > process, > >> > > > > > >> > > > > when > >> > > > > > reviewer might want to ask contributor to fix something. > >> Checklist > >> > is a > >> > > > > > different thing - this is a set of rules which must be > followed > >> by > >> > > > > > >> > > > > anyone. > >> > > > > > I do not understand how you can define documentation in this > >> > checklist. > >> > > > > > Same problem with logging - what is "enough"? > >> > > > > > > >> > > > > > On Tue, Apr 24, 2018 at 4:51 PM, Eduard Shangareev < > >> > > > > > eduard.shangar...@gmail.com> wrote: > >> > > > > > > >> > > > > > > Igniters, > >> > > > > > > > >> > > > > > > I don't understand why you are so against refactoring. > >> > > > > > > Code already smells like hell. Methods 200+ line is normal. > >> > Exchange > >> > > > > > > >> > > > > > future > >> > > > > > > is asking to be separated on several one. Transaction code > >> could > >> > > > > > > >> > > > > > understand > >> > > > > > > few people. > >> > > > > > > > >> > > > > > > If we separate refactoring from development it would mean > that > >> > no one > >> > > > > > > >> > > > > > will > >> > > > > > > do it. > >> > > > > > > > >> > > > > > > > >> > > > > > > 2) Documentation. > >> > > > > > > Everything which was asked by reviewers to clarify idea > >> should be > >> > > > > > > >> > > > > > reflected > >> > > > > > > in the code. > >> > > > > > > > >> > > > > > > 3) Logging. > >> > > > > > > Logging should be enough to troubleshoot the problem if > >> someone > >> > comes > >> > > > > > >> > > > > to > >> > > > > > > user-list with an issue in the code. > >> > > > > > > > >> > > > > > > > >> > > > > > > On Fri, Apr 20, 2018 at 7:06 PM, Dmitry Pavlov < > >> > > > > >> > > > dpavlov....@gmail.com> > >> > > > > > > wrote: > >> > > > > > > > >> > > > > > > > Hi Igniters, > >> > > > > > > > > >> > > > > > > > +1 to idea of checklist. > >> > > > > > > > > >> > > > > > > > +1 to refactoring and documenting code related to ticket > in > >> > +/-20 > >> > > > > >> > > > LOC > >> > > > > > at > >> > > > > > > > least. > >> > > > > > > > > >> > > > > > > > If we start to do it as part of our regular contribution, > >> code > >> > will > >> > > > > > >> > > > > be > >> > > > > > > > better, it would became common practice and part of Apache > >> > Ignite > >> > > > > > > > development culure. > >> > > > > > > > > >> > > > > > > > If we will hope we will have free time to submit separate > >> patch > >> > > > > > >> > > > > someday > >> > > > > > > and > >> > > > > > > > have patience to complete patch-submission process, code > >> will > >> > > > > >> > > > remain > >> > > > > > > > undocumented and poor-readable. > >> > > > > > > > > >> > > > > > > > Sincerely, > >> > > > > > > > Dmitriy Pavlov > >> > > > > > > > > >> > > > > > > > пт, 20 апр. 2018 г. в 18:56, Александр Меньшиков < > >> > > > > > >> > > > > sharple...@gmail.com > >> > > > > > > : > >> > > > > > > > > >> > > > > > > > > 4) Metrics. > >> > > > > > > > > partially +1 > >> > > > > > > > > > >> > > > > > > > > It makes sense to have some minimal code coverage for > new > >> > code in > >> > > > > > >> > > > > PR. > >> > > > > > > > IMHO. > >> > > > > > > > > > >> > > > > > > > > Also, we can limit the cyclomatic complexity of the new > >> code > >> > in > >> > > > > >> > > > PR > >> > > > > > too. > >> > > > > > > > > > >> > > > > > > > > 6) Refactoring > >> > > > > > > > > -1 > >> > > > > > > > > > >> > > > > > > > > I understand why people want to refactor old code. > >> > > > > > > > > But I think refactoring should be always a separate > task. > >> > > > > > > > > And it's better to remove all refactoring from PR, if > it's > >> > not > >> > > > > >> > > > the > >> > > > > > > sense > >> > > > > > > > of > >> > > > > > > > > the issue. > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > 2018-04-20 16:54 GMT+03:00 Andrey Kuznetsov < > >> > stku...@gmail.com>: > >> > > > > > > > > > >> > > > > > > > > > What about adding the following item to the checklist: > >> > when the > >> > > > > > > > >> > > > > > > change > >> > > > > > > > > adds > >> > > > > > > > > > new functionality, then unit tests should also be > >> > provided, if > >> > > > > > >> > > > > it's > >> > > > > > > > > > technically possible? > >> > > > > > > > > > > >> > > > > > > > > > As for refactorings, in fact they are strongly > >> discouraged > >> > > > > >> > > > today > >> > > > > > for > >> > > > > > > > some > >> > > > > > > > > > unclear reason. Let's permit to make refactorings in > the > >> > > > > > >> > > > > checklist > >> > > > > > > > being > >> > > > > > > > > > discussed. (Of cource, refactoring should relate to > >> problem > >> > > > > >> > > > being > >> > > > > > > > > solved.) > >> > > > > > > > > > > >> > > > > > > > > > 2018-04-20 16:16 GMT+03:00 Vladimir Ozerov < > >> > > > > >> > > > voze...@gridgain.com > >> > > > > > : > >> > > > > > > > > > > >> > > > > > > > > > > Hi Ed, > >> > > > > > > > > > > > >> > > > > > > > > > > Unfortunately some of these points are not good > >> > candidates > >> > > > > >> > > > for > >> > > > > > the > >> > > > > > > > > > > checklist because of these: > >> > > > > > > > > > > - It must be clear and disallow *multiple > >> > interpretations* > >> > > > > > > > > > > - It must be *lightweight*, otherwise Ignite > >> development > >> > > > > >> > > > would > >> > > > > > > > become a > >> > > > > > > > > > > nightmare > >> > > > > > > > > > > > >> > > > > > > > > > > We cannot have "nice to have" points here. Checklist > >> > should > >> > > > > > > >> > > > > > answer > >> > > > > > > > the > >> > > > > > > > > > > question "is ticket eligible to be merged?" > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 1) Code style. > >> > > > > > > > > > > > >> > > > > > > > > > > +1 > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 2) Documentation > >> > > > > > > > > > > > >> > > > > > > > > > > -1, it is impossible to define what is > >> > "well-documented". A > >> > > > > > >> > > > > piece > >> > > > > > > of > >> > > > > > > > > code > >> > > > > > > > > > > could be obvious for one contributor, and > non-obvious > >> for > >> > > > > > > >> > > > > > another. > >> > > > > > > In > >> > > > > > > > > any > >> > > > > > > > > > > case this is not a blocker for merge. Instead, > during > >> > review > >> > > > > > >> > > > > one > >> > > > > > > can > >> > > > > > > > > ask > >> > > > > > > > > > > implementer to add more docs, but it cannot be > forced. > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 3) Logging > >> > > > > > > > > > > > >> > > > > > > > > > > -1, same problem - what is "enough logging?". Enough > >> for > >> > > > > >> > > > whom? > >> > > > > > How > >> > > > > > > to > >> > > > > > > > > > > understand whether it is enough or not? > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 4) Metrics > >> > > > > > > > > > > > >> > > > > > > > > > > -1, no clear boundaries, and decision on whether > >> metrics > >> > are > >> > > > > >> > > > to > >> > > > > > be > >> > > > > > > > > added > >> > > > > > > > > > or > >> > > > > > > > > > > not should be performed during design phase. As > >> before, > >> > it is > >> > > > > > > > > >> > > > > > > > perfectly > >> > > > > > > > > > > valid to ask contributor to add metrics with clear > >> > > > > >> > > > explanation > >> > > > > > why, > >> > > > > > > > but > >> > > > > > > > > > > this is not part of the checklist. > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 5) TC status > >> > > > > > > > > > > > >> > > > > > > > > > > +1, already mentioned > >> > > > > > > > > > > > >> > > > > > > > > > > > > > 6) Refactoring > >> > > > > > > > > > > > >> > > > > > > > > > > Strong -1. OOP is a slippery slope, there are no > good > >> > and bad > >> > > > > > > > > >> > > > > > > > receipts > >> > > > > > > > > > for > >> > > > > > > > > > > all cases, hence it cannot be used in a checklist. > >> > > > > > > > > > > > >> > > > > > > > > > > We can borrow useful rules from p.2, p.3 and p.4 if > >> you > >> > > > > >> > > > provide > >> > > > > > > clear > >> > > > > > > > > > > definitions on how to measure them. > >> > > > > > > > > > > > >> > > > > > > > > > > Vladimir. > >> > > > > > > > > > > > >> > > > > > > > > > > On Fri, Apr 20, 2018 at 3:50 PM, Eduard Shangareev < > >> > > > > > > > > > > eduard.shangar...@gmail.com> wrote: > >> > > > > > > > > > > > >> > > > > > > > > > > > Also, I want to add some technical requirement. > >> Let's > >> > > > > >> > > > discuss > >> > > > > > > them. > >> > > > > > > > > > > > > >> > > > > > > > > > > > 1) Code style. > >> > > > > > > > > > > > The code needs to be formatted according to coding > >> > > > > >> > > > guidelines > >> > > > > > > > > > > > < > >> > > > > > > > > > >> > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/ > >> > > > > > >> > > > > Coding+Guidelines > >> > > > > > > > > > > . > >> > > > > > > > > > > > The > >> > > > > > > > > > > > code must not contain TODOs without a ticket > >> reference. > >> > > > > > > > > > > > > >> > > > > > > > > > > > It is highly recommended to make major formatting > >> > changes > >> > > > > >> > > > in > >> > > > > > > > existing > >> > > > > > > > > > > code > >> > > > > > > > > > > > as a separate commit, to make review process more > >> > > > > >> > > > practical. > >> > > > > > > > > > > > > >> > > > > > > > > > > > 2) Documentation. > >> > > > > > > > > > > > Added code should be well-documented. Any methods > >> that > >> > > > > >> > > > raise > >> > > > > > > > > questions > >> > > > > > > > > > > > regarding their code flow, invariants, > >> synchronization, > >> > > > > >> > > > etc., > >> > > > > > > must > >> > > > > > > > be > >> > > > > > > > > > > > documented with comprehensive javadoc. Any > reviewer > >> can > >> > > > > > >> > > > > request > >> > > > > > > > that > >> > > > > > > > > a > >> > > > > > > > > > > > particular added method be documented. Also, it > is a > >> > good > >> > > > > > > > >> > > > > > > practice > >> > > > > > > > to > >> > > > > > > > > > > > document old code in a 10-20 lines region around > >> > changed > >> > > > > > >> > > > > code. > >> > > > > > > > > > > > > >> > > > > > > > > > > > 3) Logging. > >> > > > > > > > > > > > Make sure that there are enough logging added in > >> every > >> > > > > > >> > > > > category > >> > > > > > > for > >> > > > > > > > > > > > possible diagnostic in field. Check that logging > >> > messages > >> > > > > >> > > > are > >> > > > > > > > > properly > >> > > > > > > > > > > > spelled. > >> > > > > > > > > > > > > >> > > > > > > > > > > > 4) Metrics. > >> > > > > > > > > > > > Are there any metrics that need to be exposed to > >> user? > >> > > > > > > > > > > > > >> > > > > > > > > > > > 5) TC status. > >> > > > > > > > > > > > > >> > > > > > > > > > > > Recheck that there are no new failing tests > >> > > > > > > > > > > > > >> > > > > > > > > > > > 6) Refactoring. > >> > > > > > > > > > > > > >> > > > > > > > > > > > The code should be better than before: > >> > > > > > > > > > > > > >> > > > > > > > > > > > - extract method from big one; > >> > > > > > > > > > > > - do anything else to make code clearer (don't > >> > forget > >> > > > > > >> > > > > about > >> > > > > > > some > >> > > > > > > > > > > > OOP-practise, replace if-else hell with > >> inheritance > >> > > > > > > > > > > > - split refactoring (renaming, code format) > from > >> > actual > >> > > > > > > > >> > > > > > > changes > >> > > > > > > > by > >> > > > > > > > > > > > separate commit > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > On Fri, Apr 20, 2018 at 3:23 PM, Eduard > Shangareev < > >> > > > > > > > > > > > eduard.shangar...@gmail.com> wrote: > >> > > > > > > > > > > > > >> > > > > > > > > > > > > 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 > >> < > >> > > > > > > > >> > > > > > > a...@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 < > >> > > > > > > > >> > > > > > > nizhi...@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" > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > -- > >> > > > > > > > > > Best regards, > >> > > > > > > > > > Andrey Kuznetsov. > >> > > > > > > > > > > >> > > > > >> > > > > >> > > > > >> > > > -- > >> > > > Best regards, > >> > > > Andrey Kuznetsov. > >> > > > > >> > > > > >
