You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by srdo <gi...@git.apache.org> on 2017/09/09 18:06:08 UTC
[GitHub] storm pull request #2316: STORM-2734: Fix checkstyle crash when running rele...
GitHub user srdo opened a pull request:
https://github.com/apache/storm/pull/2316
STORM-2734: Fix checkstyle crash when running release:prepare goal. A…
…ttach storm-integration-test as a module. Fix other minor issues with the POMs.
https://issues.apache.org/jira/browse/STORM-2734
Changes:
* Upgrade Checkstyle to latest version, update storm checkstyle to be based on the latest google.xml. It seems like Checkstyle got a little better at finding instances of missing Javadoc, hence the extra violations.
* Update to latest Apache parent POM. This is to resolve an issue breaking the release:prepare goal (or any build running with -Papache-release), where maven-sources-plugin:jar causes some lifecycle phases to run multiple times. For most modules this is not a problem, but storm-sql-core ends up running checkstyle (validate phase) twice per project. The second execution includes generated files in the sources directory, which causes a stack overflow in checkstyle. The latest Apache parent uses maven-sources-plugin:jar-no-fork which doesn't repeat lifecycle phases. More here http://blog.peterlynch.ca/2010/05/maven-how-to-prevent-generate-sources.html
* Remove pluginmanagement plugins also set in Apache parent POM.
* Add integration-test as a module, make it skip tests when run normally, and skip deploy during release. This way the version will be kept in sync with the rest of Storm.
* Calcite-core was declared in multiple versions in storm-hive, one compile scope, one test scope. I made it use the calcite version from the root pom.
* Fix new checkstyle violations in storm-kafka-client.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/srdo/storm attach-integration-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2316.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2316
----
commit 13122c16c8dcc9a1da46d118471f635beb4a1f07
Author: Stig Rohde Døssing <sr...@apache.org>
Date: 2017-09-07T18:47:19Z
STORM-2734: Fix checkstyle crash when running release:prepare goal. Attach storm-integration-test as a module. Fix other minor issues with the POMs
----
---
[GitHub] storm pull request #2316: STORM-2734: Fix checkstyle crash when running rele...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2316
---
[GitHub] storm issue #2316: STORM-2734: Fix checkstyle crash when running release:pre...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2316
@revans2 It's not the goal itself, it's because the apache-release profile is activated https://svn.apache.org/viewvc/maven/pom/tags/apache-10/pom.xml?view=markup#l298.
In the 10 version of the Apache parent the apache-release profile adds a run of maven-source-plugin:jar, which forks the build and runs some phases (e.g. validate) even if they've already run. This causes checkstyle to run twice on all modules, which is usually not a problem. In storm-sql-core it happens to run both before and after some code generation, which breaks the build because Checkstyle crashes when checking the generated code.
The 18 parent version switches to the jar-no-fork goal, which doesn't run checkstyle multiple times.
Thanks for reviewing btw.
---