-
Notifications
You must be signed in to change notification settings - Fork 10
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
Feature/gha builds #98
Conversation
Codecov Report
@@ Coverage Diff @@
## main #98 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 364 364
Lines 26827 26827
=======================================
Hits 26421 26421
Misses 406 406
Flags with carried forward coverage won't be shown. Click here to find out more.
This change has been scanned for critical changes. Learn more |
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.
I'd ask for at least a 2nd opinion because it's a big and complex PR this one. But I trust you and things look good to me.
Left some small comments and questions, but all very nit
docker/test.yml
Outdated
@@ -0,0 +1,63 @@ | |||
setup: |
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.
nit name this file test_codecov_config.yml
@@ -2,7 +2,7 @@ | |||
|
|||
from shared.config import get_config | |||
|
|||
HEALTH_CHECK_DEFAULT_INTERVAL_SECONDS = 10 | |||
HEALTH_CHECK_DEFAULT_INTERVAL_SECONDS = 10000 |
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.
is this not in seconds?
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.
It is. This task is .... not great. It spams the logs infinitely. I have a task for this sprint to give the option to disable it. I set that to super high locally as I was annoyed by the logs and forgot to remove. We should probably consider a more robust solution such as default disable or making it less spammy. We have it set to 100000 in prod I think 😛 .
@@ -1,5 +1,5 @@ | |||
git+ssh://[email protected]/codecov/shared.git@bef9260a4b6218b5ce4b5b9152a71d7e6d63a954#egg=shared | |||
git+ssh://[email protected]/codecov/[email protected]#egg=codecovopentelem | |||
https://github.com/codecov/shared/archive/bef9260a4b6218b5ce4b5b9152a71d7e6d63a954#egg=shared |
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.
is this how we're gonna update shared from now on?
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.
Short answer yes. Its the same setup. Just update the sha in the url.
@matt-codecov has some larger scope changes for shared in the works. However for now, moving to https allows us to not use the ssh key mounting for pip + docker. That was not necessary since becoming source available.
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.