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
Only upload 3 sets of test results to codecov (possible workaround for hanging builds) #4147
Only upload 3 sets of test results to codecov (possible workaround for hanging builds) #4147
Conversation
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #4147 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 195 195
Lines 14922 14920 -2
=========================================
- Hits 14922 14920 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Mind fixing the few coverage gaps it has now? |
@@ -41,7 +41,25 @@ jobs: | |||
runs-on: ubuntu-latest | |||
strategy: | |||
matrix: | |||
python-version: [ '3.7', '3.8', '3.9', '3.10', '3.11' ] | |||
python-version: [ '3.7', '3.8', '3.9', '3.10' ] |
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.
Would be nice to put some comment that old versions don't have coverage. So hopefully the next person, when 3.12 comes out, will know to put 3.11 in this list and put 3.12 on line 62.
@@ -120,7 +138,42 @@ jobs: | |||
- 5432:5432 | |||
strategy: | |||
matrix: | |||
dbt-version: [ 'dbt020', 'dbt021', 'dbt100', 'dbt130' ] | |||
dbt-version: [ 'dbt020', 'dbt021', 'dbt100' ] |
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.
ditto about comment to help us when bumping this version in the future
There might be a syntax like |
The |
@greg-finley: Ready for another review. Addressed everything except adding |
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.
Awesome, looks great!
Brief summary of the change made
Fixes #4146
Only require (and upload) coverage reports for:
Allegedly, uploading fewer coverage reports makes the coverage portion of the CI build less likely to hang.
Are there any other side effects of this change that we should be aware of?
Pull Request checklist
Please confirm you have completed any of the necessary steps below.
Included test cases to demonstrate any code changes, which may be one or more of the following:
.yml
rule test cases intest/fixtures/rules/std_rule_cases
..sql
/.yml
parser test cases intest/fixtures/dialects
(note YML files can be auto generated withtox -e generate-fixture-yml
).test/fixtures/linter/autofix
.Added appropriate documentation for the change.
Created GitHub issues for any relevant followup/future enhancements if appropriate.