Skip to content
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

Add more codecov configuration to fix associating commits within PRs #251

Merged
merged 3 commits into from
Jun 12, 2024

Conversation

johannaengland
Copy link
Contributor

This PR does two things to hopefully get the right coverage reports for PRs from forked repos:

  1. Add a checkout action to the workflow where uploading coverage happens, this is instructed in environment specific requirements of codecov
  2. Override the parent commit value when uploading the coverage reports by creating an artifact in the test workflow and the accessing it in the uploading workflow

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me, and I assume you've tested it in another repo.

Although I feel hesitant to spend even more time on this in order to get it right, I might suggest that we maybe should look into packaging this as a reusable action (if possible), so we don't need to repeat everything over and over in all our repos.

@johannaengland
Copy link
Contributor Author

johannaengland commented Jun 12, 2024

This looks reasonable to me, and I assume you've tested it in another repo.

Yes, it has been tested. It does not always show the correct number of commits, but it does show the correct coverage change due to direct and indirect changes and these can be inspected to figure out which lines have changed/missing coverage, which is what we need codecov for.

Although I feel hesitant to spend even more time on this in order to get it right, I might suggest that we maybe should look into packaging this as a reusable action (if possible), so we don't need to repeat everything over and over in all our repos.

I believe that that would take more time than just copy-pasting and adapting it for our other repos. Mainly because I have no experience with packaging actions and because how we do it now consists of two steps, which I don't even know if it's possible to package these into one action.

@johannaengland
Copy link
Contributor Author

Copy link
Contributor

@hmpf hmpf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My condolences. What $%^ to have in your brain...

Copy link

sonarcloud bot commented Jun 12, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@johannaengland johannaengland merged commit 8db7776 into Uninett:master Jun 12, 2024
9 of 10 checks passed
@johannaengland johannaengland deleted the workflows/add-base-sha branch June 12, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants