-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
ref: fix typing in frontend debug views #76294
base: master
Are you sure you want to change the base?
Conversation
@@ -14,7 +15,7 @@ class DebugCodeOwnersAutoSyncFailureView(View): | |||
def get(self, request: HttpRequest) -> HttpResponse: | |||
org = Organization(id=1, slug="petal", name="Petal") | |||
project = Project(id=1, slug="nodejs", name="Node.js", organization=org) | |||
user = User(name="Nisanthan") | |||
user = Actor.from_orm_user(User(name="Nisanthan")) |
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 this broken before? I'm confused about how we could start passing an actor and have this still work
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 suspect the type annotation here is incorrect (the underlying function it calls accepts User | Team | Actor
) and that it works fine with any of them. but when I went to fix that annotation it exposed a variance issue due to the way the parameters are passed (it ~essentially has def f[U: User | Team | Actor](..., user: U, by_user: dict[U, ...] | None = ...):
which I couldn't find a succinct way to make it sound for the other callers. so yeah this was the "easiest" way to fix it. the view itself only really cares about .id
so they end up being equivalent
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
c7a68c1
to
a769e96
Compare
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #76294 +/- ##
=======================================
Coverage 78.15% 78.16%
=======================================
Files 6912 6912
Lines 307213 307224 +11
Branches 50338 50340 +2
=======================================
+ Hits 240114 240130 +16
+ Misses 60700 60693 -7
- Partials 6399 6401 +2 |
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
No description provided.