APEX-186 Enable license header check in Travis CI. #106
Conversation
|
@vrozov please review |
| @@ -15,6 +15,8 @@ | |||
|
|
|||
| language: java | |||
|
|
|||
| script: mvn clean verify -Dlicense.skip=false | |||
There was a problem hiding this comment.
Do we need to run verify? By default travis-ci uses "mvn test". Should it be "mvn license:check test -Dlicense.skip=false"?
Are the following warnings expected?
[WARNING] Unknown file extension: /Users/vrozov/Projects/DataTorrent/apex/NOTICE
[WARNING] Unknown file extension: /Users/vrozov/Projects/DataTorrent/apex/LICENSE
[WARNING] Unknown file extension: /Users/vrozov/Projects/DataTorrent/apex/DISCLAIMER
[WARNING] Unable to find a comment style definition for some files. You may want to add a custom mapping for the relevant file extensions.
|
Yes, we want to run verify, which includes test as well as apache-rat:check and other checks that need to occur. The 3 files that have a warning need to be added to the exclusion list. |
|
apache-rat:check by default binds to validate phase. Did we change it to run during verify? Will you add 3 files to the exclusion list or they were already added to the exclusion list as part of a different commit/pull request? |
|
Added the 3 files to exclusion list. |
|
These checks all run after test (japicmp, license, rat). That's good, don't want these to run on every iteration during development. |
|
japicmp and maven license checks do run during verify. checkstyle plugin runs during validate as well as enforcer. apache-rat does not run at all unless explicitly invoked by "mvn apache-rat:check". Should apache-rat be enabled for automatic run during verify? I think that we need to have it enabled, so that committers can execute "mvn clean verify" and not to wary that not all checks are passed. Will it be better to fail build due to license or checkstyle violation before tests are executed as tests take most of the time? If developer/contributor wants to skip license/checkstyle validation, can they do this using command line parameters/properties/flags? |
|
checkstyle can be skipped by : mvn clean install -Dcheckstyle.skip We can also run mvn with fail-never option to see all the failures at the end. Don't mean to divert the discussion but I think we can reduce the no. of plugins by getting rid of license check. License can be validated by checkstyle as well. |
|
@ChandniSingh This change is about what runs in Travis CI. The license header check does not run in the development environment unless you do the -Dlicense.skip=false @vrozov Good catch, I'm going to add the apache-rat:check as a goal to the Travis CI script. I would prefer to keep the "main loop" of the development build light. Things like japicmp and license header check add to the build time, they can come late in the build lifecycle. The important thing is these issues are caught in the PR CI. |
|
Personally I don't mind few seconds extra time for checkstyle and license check during main development lifecycle as test consumes most of the time and I don't want to rerun tests after fixing code style or missing/incorrect license. At the same time, if license and code style templates are integrated into IDE, those checks are redundant most of the time. Can we get more attention to https://malhar.atlassian.net/browse/APEX-137 and define workflow for contributors/committers. It looks that Travis CI status is ignored (https://travis-ci.org/apache/incubator-apex-malhar/builds). It will be nice to have something like https://issues.apache.org/jira/browse/HDFS-9181?focusedCommentId=14946362&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14946362. |
|
Made the script change. +1 for attention to APEX-137 :-) |
APEX-29 #resolve Use DefaultEventLoop.createEventLoop factory
No description provided.