-
Notifications
You must be signed in to change notification settings - Fork 28
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
chore: Cleanup unused graphql fields - coverageAnalytics #843
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #843 +/- ##
==========================================
- Coverage 96.26% 96.25% -0.01%
==========================================
Files 814 814
Lines 18680 18659 -21
==========================================
- Hits 17982 17961 -21
Misses 698 698
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
a5cf820
to
82502b9
Compare
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.
These tests are now at test_coverage_analytics_measurements.py
@@ -62,31 +60,6 @@ def resolve_oldest_commit_at( | |||
return None | |||
|
|||
|
|||
@repository_bindable.field("coverage") |
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.
These fields are now within the coverageAnalytics
type at coverage_analytics.py
. Frontend has been cut over with codecov/engineering-team#2281
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.
Note that the type check CI step fails as I did not attempt to convert the repository.py
file to include types with this ticket
assert self.fetch_repository( | ||
repo.name, | ||
default_fields | ||
+ "coverageAnalytics { percentCovered commitSha hits misses lines },", |
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 it possible to add the update the default_fields object with this new object instead of removing it and supplementing the query here?
Or would that have meant we needed to update more tests or something
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 thought these fields aren't necessarily "default" as we extend the repository object with more stuff and maybe more product lines into the future. I'd guess net new tests would more likely want to test repository without coverageAnalytics related fields (& those could get tested within the test_coverage_analytics.py area)
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.
looks good
Remove now-unused GraphQL fields for GraphQL restructure project.
Related to these tickets:
api
PR duplicating the fields - feat: Add graphql coverage analytics type #806gazebo
PRs pointing over to the new fields - feat: Use new coverage analytics graphql schema gazebo#3169, fix: Use coverageAnalytics new graphql fields gazebo#3339Closes codecov/engineering-team#2282
Tested on staging by confirming no regression after removing these fields