Hi Enrico, Thanks for your participant!
Running after turn on checkstyle on zookeeper-server it reports about 9000 checkstyle failures(see failures.txt[1]) among 596 files(see files.txt[1]). Most of them are related to whitespace and import. I think I can handle it with one or two whole days but I'm afraid that I have no spare time until next weekend(08.17). Also I would prefer make it into one big PR and separate commits per file, i.e., resolving failures per file for smoothly review. Best, tison. [1] https://gist.github.com/TisonKun/ba69524defbf3e9d4c99eaf2de338e5a Zili Chen <wander4...@gmail.com> 于2019年8月6日周二 上午9:43写道： > Hi Enrico, > > Thanks for your participant! > > Running after turn on checkstyle on zookeeper-server > it reports about 9000 checkstyle failures(see out.txt) > among 596 files(see conflicts.txt). Most of them > are related to whitespace and import. > > I think I can handle it with one or two whole days but > I'm afraid that I have no spare time until next weekend(08.17). > > Also I would prefer make it into one big PR and separate > commits per file, i.e., resolving failures per file for > smoothly review. > > Best, > tison. > > > Enrico Olivelli <eolive...@gmail.com> 于2019年8月5日周一 下午9:37写道： > >> Il giorno lun 5 ago 2019 alle ore 14:03 Zili Chen <wander4...@gmail.com> >> ha >> scritto: >> >> > Hi zookeepers, >> > >> > Recently I've sent a pr[1] on enable checkstyle configuration on >> > zookeeper-promethus-metrics, Enrico(committer) and Dinesh（contributor) >> > kindly reviewed it but we still need another look from committer as >> > process required. Someone of you has spare time to have a look? >> > >> >> Tison, >> Norbert approved the PR and I have merged it. >> We have already begun this check style work. >> Thank you so much to you and Mao Ling for these efforts. >> >> >> > >> > Besides, as ZOOKEEPER-3431 described, we're going to enable this >> > configuration on zookeeper-server as following. This is really a major >> > package which contains a lot of codes involved by multi prs. I'd like to >> > hear from the community about how to proceed. >> > >> >> I think that if we do it quickly there will be as few troubles as >> possible. >> I am available for review, just ping me. >> >> >> >> > >> > (From where I stand, enable on the whole package at once >> > could solve the problem at once, but it would conflict quite a lot >> > ongoing prs. On the other hand, sending the pr without timely reviews >> > also causes a burden to rebase it on the master and solve conflict >> > again and again(since it likely conflicts with a merged pr)) >> > >> >> Did you check how many violations we have con zookeeper-server ? >> How much big is the work ? >> Do you have an estimate ? >> >> We could take into consideration to have only one big PR. >> The tradeoff is that it will be very difficult to perform a review, we >> will >> need more eyes on the diff >> >> Enrico >> >> >> >> >> > >> > Best, >> > tison. >> > >> > [1] https://github.com/apache/zookeeper/pull/1028 >> > >> >
