ci: add license validation step for core and dev modules #183
Conversation
|
@xinhuagu this could be useful for #168, but are you sure that this will actually work? This PR basically adds |
@vorburger It will build. The "check" goal was directly invoked with the checkstyle.header.file parameter, even without explicit configuration in the pom.xml. It still works, however, it is the minimal change, not the optimal one. I updated the PR accordingly,thanks! |
| - name: License validation for ${{ matrix.module }} | ||
| working-directory: ./${{ matrix.module }} | ||
| run: mvn checkstyle:check -Dcheckstyle.config.location=../license-checks.xml |
There was a problem hiding this comment.
Let's not do this like this (i.e. running Maven twice using a Matrix), but instead wait for #155 to get merged (hopefully in the next few days already), and then simply use standard Maven? (Once there is a parent POM, Checkstyle can run on all modules.)
There was a problem hiding this comment.
Sure, using a parent POM is definitely the better solution
| <java-websocket.version>1.6.0</java-websocket.version> | ||
| <jackson.version>2.19.0</jackson.version> | ||
| <okhttp.version>4.12.0</okhttp.version> | ||
| <checkstyle-maven-plugin.version>3.6.0</checkstyle-maven-plugin.version> |
There was a problem hiding this comment.
Let's wait for #155 and then add this to the (new) root pom.xml instead of duplicating it in both the core/pom.xml as well as the dev/pom.xml ?
There was a problem hiding this comment.
Agree, once the root POM is there, there's no need to repeat the version in multiple places.
| <artifactId>maven-checkstyle-plugin</artifactId> | ||
| <version>${checkstyle-maven-plugin.version}</version> | ||
| <configuration> | ||
| <propertyExpansion>checkstyle.header.file=../java.header</propertyExpansion> |
There was a problem hiding this comment.
I'm curious about something that I have not yet understood: Will this ONLY check the java.header for the license? That would be a great! Or will this ALSO validate "code format"? That would a problem. Because the code in this repo will never (exactly) match Checkstyle code conventions. (Instead, I will add another code format checker later, see #168.) Could you double check to clarify? Tx!
There was a problem hiding this comment.
this checkstyle.header.file=../java.header will also valide other default rules. That would be a problem , true. Thx for pointing it out.
looking forward to #168 , i close this pr now :) thx
2 participants
Description
adds automated license validation steps in CI
( Hello, If the java.header and license-checks files are automatically used elsewhere or have other uses and it shouldn't be validated in ci, please close this PR. Thanks )