gortiz commented on code in PR #10528: URL: https://github.com/apache/pinot/pull/10528#discussion_r1211261931
########## .github/workflows/scripts/.pinot_test.sh: ########## @@ -79,7 +83,9 @@ else -pl '!:pinot-csv' \ -pl '!:pinot-json' \ -pl '!:pinot-segment-uploader-default' \ - -P github-actions,no-integration-tests && exit 0 || exit 1 + -P github-actions,no-integration-tests \ + -Dspotless.apply.skip -Dcheckstyle.skip -Dspotless.apply.skip -Dlicense.skip=true \ Review Comment: There are 3 reasons: 1. It is not actually needed. These are already verified by the `linter` GA job, so we are not going to miss them. 2. Subjectively, it is problematic. With this changes, tests are going to be executed even when something with linter errors is pushed. That means that only linter fails (ie I usually forget to add the headers) but tests pass, you can be confident than once the linter is fixed, everything should go green. 3. Objectively, there were problems when not skipping them. Mainly, spotless doesn't work right now in Java 20. Therefore the change was necessary to run the tests. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org