> On 18 Feb 2020, at 19:04, Gary Gregory <garydgreg...@gmail.com> wrote: > > At issue is that each Commons components has its own style and sometimes > its own custom Checkstyle configuration in its own location. My preference > is: > - Store a default Checkstyle, SpotBugs, JApiCmp, and JaCoCo configuration > in Commons Parent under src/site/resources > - Avoid bikeshedding for what should be in these above files by picking > Commons Lang's versions of these files. > - Since Commons Collections does not define a Checkstyle, it can inherit it > from Commons Parent > - Other components can migrate to the Commons Parent configurations at a > time of their own choosing. > Gary
Collections does have a checkstyle config, it is just very old. It is a lot more manageable using the config from [lang] than the strict one adapted from [numbers]. With the current checkstyle version there are 157 errors. If I upgrade checkstyle to a more recent version (plugin 3.1.0 and dependency 8.29) then there are 177 errors. For a start I’ll grab the [lang] config and fix the code for that. The config from [lang] misses indentation checks and some whitespace checks so I can do these in round 2. As for bundling this with parent then I think it may require a different project to package all the commons build resources into a jar to maven central. The jar can then be included as a dependency within the checkstyle (etc) configuration or added to the <build> extensions to make it available on the maven classpath [2]. This config could be put into commons-parent. So should this be a new project commons-build-resources? [1] https://maven.apache.org/ref/3.6.3/maven-model/maven.html#class_extension <https://maven.apache.org/ref/3.6.3/maven-model/maven.html#class_extension> [2] https://maven.apache.org/guides/mini/guide-maven-classloading.html#Core_Classloader <https://maven.apache.org/guides/mini/guide-maven-classloading.html#Core_Classloader> > > On Tue, Feb 18, 2020 at 1:43 PM Alex Herbert <alex.d.herb...@gmail.com> > wrote: > >> There have been a few PRs recently in collections with simple formatting >> errors. These should be picked up by checkstyle to prevent correction >> after merge. >> >> The [collections] checkstyle configuration is old. If I replace it with >> the Checkstyle version close to the Sun standard [1] then there are a >> lot of errors (>3000). With a few rule changes to ignore items the >> number of errors is down to about 2000. A lot of these are things that >> should be corrected and would be good to have to filter PRs: >> >> JavadocMethod 188 >> >> JavadocStyle 517 >> >> JavadocType 186 >> >> JavadocVariable 177 >> >> WhitespaceAround 86 >> >> WhitespaceAfter 68 >> >> RedundantModifier 40 >> >> Indentation 177 >> >> Header 31 (for the Apache licence) >> >> I suggest updating the checkstyle config to at least catch these. I can >> either use the config that flagged these errors and remove rules that >> are not enforced or add these rules to the current config. >> >> I'm thinking a total refresh of the config and then removal of items >> that are not to be enforced is the way to go. These can gradually be >> reintroduced as the code gets fixed over time (because I don't think I >> will be able to do it all any time soon). >> >> Any opinions on this effort? >> >> Alex >> >> [1] >> >> https://github.com/checkstyle/checkstyle/blob/master/src/main/resources/sun_checks.xml >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> >>
