-
Notifications
You must be signed in to change notification settings - Fork 22
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
[DEPR]: Codecov #429
Comments
I'm fully in favor of removing a dependency on a 3rd party service that doesn't work (and manages to break our builds , too), but enforcing code coverage still strikes me as being as important as it's always been. Rather than totally ripping out the coverage check, could we set it up to enforce some generally-reasonable threshhold by checking the output of the coverage.py? Teams could still lower the threshhold or rip it out, but at least they'd be start out with a reasonable default coverage check. |
Updated description with a possible alternative: https://github.com/py-cov-action/python-coverage-comment-action |
I'd really like this to get someone's attention, because the failing uploads that breaks PRs is a frustration that many people are probably feeling. I guess we have scripts where we might be able to quantify. I think this issue deserves a shorter term solution, which either means someone [Axim? Community? Arch-BOM?] picking up this work so the long term is now, or implementing a retry or some other purely short term solution. Thoughts? |
As I understand it:
Given that the short- and medium-term solutions only benefit new repositories, my inclination is that those solutions are not high-priority for Axim to tackle ourselves (although, to be clear, I have no opposition to those solutions). Right now, our most acute challenge is ensuring a state of good maintenance for all Core Product repositories, which is already an overwhelming list. |
@kdmccormick: I see. I didn't realize this issue was in edx-cookiecutters, and I was really asking about existing repos, like openedx-events where I saw a recent codecov upload failure.
So I guess for openedx-events, the options is to disable the check altogether, or use something else? Is there an example of the something else? This ticket lists a hackathon project as a potential replacement, but I'm not sure how far that got. Ideally we would not disable altogether without a replacement. I imagine there must be more repos with this problem. |
We actually added documentation to the developer docs on how to set up and use a CodeCov alternative: https://docs.openedx.org/en/latest/developers/how-tos/use-python-coverage-comment.html 2U uses it in one of our private repos and it seems to be working acceptably. |
@robrap Ah, I was confused too. I didn't realize that this was an active issue for existing repos. Those docs look great @dianakhuang ! Would you both agree that:
|
This has passed the acceptance date, and I think we're all in agreement that we should move off CodeCov at least by default. |
Note that the ticket edx/edx-arch-experiments#528 to implement an alternative got blocked/abandoned due to 2U organization issues. |
openedx-events is attempting to use the replacement here: openedx/openedx-events#323 |
We don't yet have a good plan for replacing JS coverage, and we need to figure out if we'll use codecov's bundle asset size tooling or if we need a different alternative. |
Proposal Date
2023-12-19
Target Ticket Acceptance Date
2023-01-10
Earliest Open edX Named Release Without This Functionality
Redwood 2023-04
Rationale
Codecov does not work OOTB for most repositories created using this cookiecutter and most people remove it once their repository has been created. There are known alternatives that do not require additional steps for setup.
Removal
edx-cookiecutters/cookiecutter-xblock/{{cookiecutter.repo_name}}/.github/workflows/ci.yml
Line 39 in 8180c89
edx-cookiecutters/cookiecutter-django-app/{{cookiecutter.repo_name}}/.github/workflows/ci.yml
Line 39 in 8180c89
Replacement
We have written up a HowTo on using the GitHub Action python-coverage-comment-action.
Deprecation
Should not be needed
Migration
Should not be needed
Additional Info
No response
The text was updated successfully, but these errors were encountered: