-
Notifications
You must be signed in to change notification settings - Fork 90
maintenance: send coverage (jacoco) to codecov #1094
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
Changes from 9 commits
d40d9e5
664bba6
b02981e
b70ac5d
2bde484
39140f7
c15c102
0fbe125
81e708a
582631d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,7 +50,7 @@ jobs: | |
name: Java ${{ matrix.java }} | ||
env: | ||
OS: ${{ matrix.os }} | ||
JAVA: ${{ matrix.java-version }} | ||
JAVA: ${{ matrix.java }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not clear to me what the previous version of this was doing ..... shouldn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so. Not sure the environment variable is even used actually... |
||
AWS_REGION: eu-west-1 | ||
steps: | ||
- uses: actions/checkout@v3 | ||
|
@@ -59,8 +59,14 @@ jobs: | |
with: | ||
distribution: 'zulu' | ||
java-version: ${{ matrix.java }} | ||
cache: 'maven' | ||
- name: Build with Maven | ||
run: mvn -Pbuild-without-spotbugs -B package --file pom.xml | ||
- name: Upload coverage to Codecov | ||
uses: codecov/codecov-action@d9f34f8cd5cb3b3eb79b3e4b5dae3a16df499a70 # 3.1.1 | ||
if: ${{ matrix.java == '11.0.x' }} # publish results once | ||
scottgerring marked this conversation as resolved.
Show resolved
Hide resolved
|
||
with: | ||
files: ./powertools-cloudformation/target/site/jacoco/jacoco.xml,./powertools-core/target/site/jacoco/jacoco.xml,./powertools-idempotency/target/site/jacoco/jacoco.xml,./powertools-logging/target/site/jacoco/jacoco.xml,./powertools-metrics/target/site/jacoco/jacoco.xml,./powertools-parameters/target/site/jacoco/jacoco.xml,./powertools-serialization/target/site/jacoco/jacoco.xml,./powertools-sqs/target/site/jacoco/jacoco.xml,./powertools-tracing/target/site/jacoco/jacoco.xml,./powertools-validation/target/site/jacoco/jacoco.xml | ||
savepr: | ||
runs-on: ubuntu-latest | ||
name: Save PR number if running on PR by dependabot | ||
|
@@ -72,7 +78,7 @@ jobs: | |
echo ${{ github.event.number }} | ||
echo ${{ github.event.number }} > ./pr/NR | ||
- uses: actions/upload-artifact@v2 | ||
name: Updload artifact | ||
name: Upload artifact | ||
with: | ||
name: pr | ||
path: pr/ |
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.
Only related to change insofar as
matrix.java-version
was also not defined in the matrix context. In this case I thinkmatrix.os
has been taken from the matrix examples but is being misused here because it is never set.I suggest removing this. I can see no usages of
OS
within the codebase, and this is actively confusing. Adding the filter on the matrix to ensure codecov runs once adds to this - if we actually had a 2D matrix with OS as well, which this implies, the filter would not be enough.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.
yep, will try to remove the os to see if it still works.