-
Notifications
You must be signed in to change notification settings - Fork 29
API Improvements: Fix typings for various files pt1 #1040
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
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 #1040 +/- ##
=======================================
Coverage 96.00% 96.01%
=======================================
Files 828 828
Lines 19397 19429 +32
=======================================
+ Hits 18623 18655 +32
Misses 774 774
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
| self._log_updated([owner]) | ||
|
|
||
| def post(self, request: HttpRequest, *args, **kwargs) -> Response: | ||
| def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> Response: |
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.
Could we know what the args/kwargs are from stripe?
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.
The code doesn't use args/kwargs at all, I think we just add it in as part of the DRF APIView interface.
services/profiling.py
Outdated
| return [] | ||
| commit_sha = self.commit_sha or profiling_commit.commit_sha | ||
|
|
||
| if profiling_commit is not None: |
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.
Was there something wrong with the one on the left? Or is this mostly preference?
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.
Hmm good point this change is actually not equivalent, old code gives self.commit_sha preference whereas as this gives profiling_commit.commit_sha preference. I need to do it like this:
commit_sha = None
if self.commit_sha:
commit_sha = self.commit_sha
elif profiling_commit:
commit_sha = profiling_commit.commit_sha
mypy didn't like the old style because profiling_commit might be null so it can produce an error
adrian-codecov
left a comment
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.
Left comments on the ones I had questions on. Tyvm, this is a great improvement
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.