[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/285 Thank you for updating the pull request. Tested locally. Using the changes in the pull request I get the same result as we get when the settings are in the Maven settings. Commenting out the suppression filter in the checkstyle configuration, the report in the web site now gives me **2828** errors. So the suppression filter is working indeed. Looks good to me, merging it now. ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user osiegmar commented on the issue: https://github.com/apache/commons-lang/pull/285 > Could you update the pull request with your original change duplicated in that portion, please? Done. Thanks for pointing that out! ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/285 [](https://coveralls.io/builds/13196430) Coverage remained the same at 95.201% when pulling **67830fe24964b0164d0212cbb4f018f15608ff37 on osiegmar:portable-checkstyle-config** into **fdf05fa29babe21e64f9a5b268dc8406d449d4f1 on apache:master**. ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/285 [](https://coveralls.io/builds/13196392) Coverage decreased (-0.001%) to 95.2% when pulling **8b21045bac1d5148ea18650313a32bed3a0e167a on osiegmar:portable-checkstyle-config** into **fdf05fa29babe21e64f9a5b268dc8406d449d4f1 on apache:master**. ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/285 >Does it really generates a site for the current branch? It should. That is one step in the release process. Probably due to the site using another configuration (under the reporting tag). https://github.com/osiegmar/commons-lang/blob/01a820275fc7bd66cb699dd081a432c7dde2645e/pom.xml#L711 Could you update the pull request with your original change duplicated in that portion, please? ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user osiegmar commented on the issue: https://github.com/apache/commons-lang/pull/285 I haven't checked the configuration of the maven site plugin in detail. Does it really generates a site for the current branch? I doubt that, as I've seen subversion commands in the config... Anyway, to run checkstyle, just invoke `mvn clean checkstyle:check`. ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/285 Not sure. Here's what I just tried. ``` $ git reset --hard $ git status On branch pr/285 $ vim checkstyle.xml $ # removed line $ mvn clean site ``` And then opening the Checkstyle report (via file:///home/.../commons-lang/target/site/index.html) there are no warnings or error in the checkstyle page. Are you able to spot which step I am missing to reproduce it? ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user osiegmar commented on the issue: https://github.com/apache/commons-lang/pull/285 Removing the SuppressionFilter results in thousands of errors. Maybe you have a stale checkstyle cache file `target/cachefile`? ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user kinow commented on the issue: https://github.com/apache/commons-lang/pull/285 Checked out pull request branch, built with the change, got no checkstyle errors. Then removed the suppression (without re-adding the previous value in pom.xml) and also got the same result, without any errors. Maybe it is aware that the suppression file must be ignored? +1 for the change. Will wait for others to review :+1: Documentation about the SuppressionFilter module. http://checkstyle.sourceforge.net/config_filters.html#SuppressionFilter Thank you for your contribution @osiegmar ---
[GitHub] commons-lang issue #285: make checkstyle config more portable (no maven coup...
Github user coveralls commented on the issue: https://github.com/apache/commons-lang/pull/285 [](https://coveralls.io/builds/13181719) Coverage remained the same at 95.194% when pulling **01a820275fc7bd66cb699dd081a432c7dde2645e on osiegmar:portable-checkstyle-config** into **cc94767e7eabdfcf9d1cab1d8d1d8556864394c6 on apache:master**. ---
