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 'instrumentkit' as owner requirement to workflow #343

Merged
merged 2 commits into from
Mar 29, 2022

Conversation

trappitsch
Copy link
Contributor

Updating my fork by merging the updated branch into it seems to run the tests now. Static checks pass, however (and of course), the actual tests fails when trying to push to codecov.

Here, the proposed change is to check if the owner is instrumentkit and if so, run the tests, otherwise not. Alternatively, we probably could only make the codecov push owner dependent and keep the tests, running even on forks.

@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #343 (bde1d13) into main (f704663) will not change coverage.
The diff coverage is n/a.

❗ Current head bde1d13 differs from pull request most recent head f17e2b6. Consider uploading reports for the commit f17e2b6 to get more accurate results

@@           Coverage Diff           @@
##             main     #343   +/-   ##
=======================================
  Coverage   99.48%   99.48%           
=======================================
  Files         174      174           
  Lines       18873    18873           
=======================================
  Hits        18776    18776           
  Misses         97       97           
Flag Coverage Δ
unittests 99.48% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@scasagrande
Copy link
Contributor

Yeah I think that it would still be useful for people to run the tests on their own forks so they can get CI feedback without opening a PR. I think its better to instead to move this if statement to uploading the reports to codecov

@trappitsch
Copy link
Contributor Author

trappitsch commented Mar 29, 2022

Good point. Moved the conditional down to the codecov upload. Passes fine now in here as well as on my fork, see trappitsch#1

@scasagrande scasagrande merged commit 722781a into instrumentkit:main Mar 29, 2022
@trappitsch trappitsch deleted the workflow_fix branch March 29, 2022 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants