-
Notifications
You must be signed in to change notification settings - Fork 0
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
#11: Introduce code coverage measuring by JaCoCo #12
Conversation
Co-authored-by: miroslavpojer <miroslav.pojer@absa.africa>
- JaCoCo introduced into project.
Updated year.
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.
It would be good to document some usages / supported types etc into README, but otherwise really good work, I like the Either
approach you chose, apart from many other things :)
Oh wait a minute, this is almost an exact copy of https://github.com/AbsaOSS/balta/pull/10/files. I propose to repoint this PR to that branch then, not to master. |
Yeah, @miroslavpojer took my branch, added JaCoco support and created a PR. But directed it to |
Hi, sorry for that. I have repointed it to from master. |
…ository' into feature/add_jacoco_support
* Updated README.md to provide hint how to run JaCoCo code coverage tooling.
Documented in README.md in commit f897094, |
project/plugins.sbt
Outdated
def armUrl(scalaMajor: String): String = s"https://repo1.maven.org/maven2/com/jsuereth/scala-arm_$scalaMajor/$scalaArmVersion/scala-arm_$scalaMajor-$scalaArmVersion.jar" | ||
|
||
addSbtPlugin("com.jsuereth" %% "scala-arm" % scalaArmVersion from armUrl("2.11")) | ||
addSbtPlugin("com.jsuereth" %% "scala-arm" % scalaArmVersion from armUrl("2.12")) |
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.
those scala-arm
- are they actually used?
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 can see that they are indirectly - they are needed for jacoco itself.
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.
Yes, They are needed for current solution. I guess it could be done better, but I still hope I will be able to refactor all this solution.
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.
Actually I noticed that the GH Pipeline definition for adding code coverage into PRs is missing.
LGTM otherwise
You are right, I will add it.
I would follow the second option as depends on the job on build.yml. I have prepared it already. |
JaCoCo code coverage report - balta
|
…_jacoco_support # Conflicts: # README.md # balta/src/main/scala/za/co/absa/db/balta/classes/setter/SetterFnc.scala # build.sbt
JaCoCo 'balta' module code coverage report - scala 2.12.18
|
JaCoCo introduced into project.
Closes #11