Hi Michael, After thinking about it, I think enable checkstyle configuration is only about formatting. And I would make sure that there is no refactor. And forked repo maintainers can rebase their work on the new master just by manually applying patches. If the forked repo has been quite diverged from master of apache/zookeeper, a better way for syncing is to cherry-pick commits from apache/zookeeper and just ignore this checkstyle stuff.
Best, tison. Zili Chen <wander4...@gmail.com> 于2019年8月8日周四 上午9:20写道： > @Michael > > FYI, it is possible by properly setting and updating suppressing rules. > > Best, > tison. > > > Michael Han <h...@apache.org> 于2019年8月8日周四 上午9:12写道： > >> I think a good approach is doing this incrementally. Doing this all at >> once >> over entire server code base will make life much harder for those who >> maintain their own ZooKeeper forks and has internal patches, and the >> change >> itself will be impossible to review (even though most changes are >> mechanical, we still need review the change). >> >> On Mon, Aug 5, 2019 at 10:57 PM Zili Chen <wander4...@gmail.com> wrote: >> >> > >I see some javadoc related issues in your gist, >> > >didn't we disable that rule ? JavadocType >> > >> > The failures reported are "Missing javadoc", and >> > what we disabled is "Empty javadoc". Fair enough to >> > disable "Missing javadoc" for now and filed into >> > ZOOKEEPER-3469 since a empty javadoc is also a >> > placeholder. >> > >> > >Are you using some automatic tool ? >> > >> > Yes, IntelliJ helps a lot. >> > Best, >> > tison. >> > >> > >> > Enrico Olivelli <eolive...@gmail.com> 于2019年8月6日周二 下午1:13写道： >> > >> > > Il giorno mar 6 ago 2019 alle ore 03:51 Zili Chen < >> wander4...@gmail.com> >> > > ha >> > > scritto: >> > > >> > > > 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 see some javadoc related issues in your gist, >> > > didn't we disable that rule ? JavadocType >> > > >> > > >> > > > 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). >> > > > >> > > >> > > Are you using some automatic tool ? >> > > In the past few months in my company I have been working in adding >> > > checkstyle to other projects and >> > > we used Apache NetBeans to fix up the code, I guess IntelliJ can do >> the >> > > same. >> > > >> > > Enrico >> > > >> > > >> > > > >> > > > 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 >> > > > >> > >> > > > >> >> > > > > >> > > > >> > > >> > >> >
