-
-
Notifications
You must be signed in to change notification settings - Fork 669
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 deepsource code coverage #5717
Add deepsource code coverage #5717
Conversation
✅ Deploy Preview for inventree-web-pui-preview canceled.
|
The function of this depends on a secret being set, that was transferred on another medium |
@SchrodingersGat this would be ready for merge |
What is the improvement? Does this mean we can drop coveralls? |
We have a full view of missing coverage in the SAST solution. I would not drop coveralls, we have years of history there. |
Makes sense. |
…deepsource-coverage
You can also commit without changing something with the --allow-empty argument |
Yes but that wouldn't trigger a server CI run. |
Not sure why this failed - |
Secrets are not used for pull_request triggers from forks for security reasons. Otherwise anyone could just read out every secret by sending them to their servers via curl when modifying the workflow. @matmair needs to set this secret in his repo and run the actions on his repo. |
Ah, of course! |
But if the action relies on this secret, I guess we need to search for a different approach? How does the coveralls auth works? |
I think coveralls relies on a github integration which you can authorize - rather than a repo secret |
I have added the DSN to my repo and it worked in my out-of-tree repos but I will just disable it on PRs |
But where is the improvement then, if this can only upload the test results on a merge but not while the pr is going on? |
I think we have a differing understanding of the goal here: Mine is to add code coverage indicators to the SAST tool. This is not meant as a replacement for coveralls. |
Oh ok, makes sense now. |
@matmair is this OK to merge in now, or needs further work? |
@SchrodingersGat this is ok to merge |
This PR is a follow-up tp #5714 and adds code coverage reporting to deepsource (the coverage used for coveralls is reused)