-
Notifications
You must be signed in to change notification settings - Fork 12
add ci-reporting, errorprone profile and record static code analysis #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…xtraCmd to pass extra arguments to pass to mvn cli
vars/asfMavenTlpPlgnBuild.groovy
Outdated
def failFast = false; | ||
def siteJdks = params.containsKey('siteJdk') ? params.siteJdk : ['8','17'] | ||
def siteMvn = params.containsKey('siteMvn') ? params.siteMvn : '3.6.x' | ||
def siteJdks = params.containsKey('siteJdk') ? params.siteJdk : ['8'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be at minimum 11, there are difference for javadoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uhm not sure as javadoc can fail with jdk 1.8 and not 11. and the other way around :)
as we are 1.8 for most of the plugins etc maybe we need to ensure it works with this at least (as release folks might use 1.8 to build their release.
vars/asfMavenTlpPlgnBuild.groovy
Outdated
// cmd += 'sonar:sonar' | ||
} | ||
if (Integer.parseInt(jdk) >= 11 && !taskContext['ciReportingRunned']) { | ||
cmd += "-Pci-reporting -Perrorprone" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ci-reporting
, errorprone
are is some new profiles, I didn't see those before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test branch ci-reporting
with maven-compiler-plugin
recordIssues id: "${os}-jdk${jdk}", name: "Static Analysis", aggregatingResults: true, enabledForFailure: true, tools: [mavenConsole(), java(), checkStyle(), spotBugs(), pmdParser(), errorProne()] | ||
jacoco inclusionPattern: '**/org/apache/maven/**/*.class', | ||
exclusionPattern: '', | ||
execPattern: '**/target/jacoco.exec', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jacoco-it.exec
should also be included
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point to include IT test in coverage.
I have setup this to be in the same file with apache/maven-compiler-plugin@7e83e0d and covered lines increased from build #16 here https://ci-maven.apache.org/job/Maven/job/maven-compiler-plugin-test-olamy/job/ci-reporting/
@olamy do You think it's still needed? |
do not maven.test.failure.ignore per default as we do not even recommend using it (https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#testFailureIgnore)