-
Notifications
You must be signed in to change notification settings - Fork 63
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
PHOENIX-6836: Enable code coverage reporting to SonarQube in Phoenix-Queryserver #112
Conversation
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.
I am not sure about the following parts, (it doesn't mean they are wrong).
If you think it is correct please provide some details for me.
<sonar.dynamicAnalysis>reuseReports</sonar.dynamicAnalysis> | ||
<sonar.surefire.reportsPath>${project.build.directory}/surefire-reports</sonar.surefire.reportsPath> | ||
<sonar.coverage.jacoco.xmlReportPaths> | ||
${project.build.directory}/site/jacoco/jacoco.xml |
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.
Why don't we have phoenix-queryserver-assembly
here like for the phoenix main repo?
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.
If I do it like in the phoenix component and aggregate the coverage in the assembly module then the phoenix-queryserver-orchestrator module is not included in the coverage however I tried to modified the configuration of the jacoco and sonar profiles.
THREADS=1 | ||
fi | ||
|
||
mvn -B -e -f "$MAIN_POM" clean verify -Pcodecoverage -fn -Dmaven.javadoc.skip=true -DskipShade -T "$THREADS" |
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.
Shouldn't we have coverage
profile here?
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 regarding the new profile I have added but never used :)
I added the sonar analysis to the original codecoverage profile so my coverage profile is not necessary, looks like I forgot to remove it.
944c5ea
to
f031cd7
Compare
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.
LGTM
No description provided.