Add some type hints for djblets.extensions.
Review Request #13074 — Created May 30, 2023 and submitted
This isn't a complete typing for the extensions module, but it's enough
to quash a lot of the warnings that show up when working on extensions
code.
Ran unit tests.
| Summary | ID |
|---|---|
| 8a428af82d7691f210ddd061f54cfbbb97955860 |
| Description | From | Last Updated |
|---|---|---|
|
Typo in the description: "exetnsions" |
 chipx86
|
|
|
line too long (82 > 79 characters) Column: 80 Error code: E501 |
 reviewbot
|
|
|
missing whitespace around parameter equals Column: 30 Error code: E252 |
reviewbot
|
|
|
'types.ModuleType' imported but unused Column: 1 Error code: F401 |
reviewbot
|
|
|
Swap these lines. Also, the typing imports aren't alphabetized (TYPE_CHECKING should be before Type). |
chipx86
|
|
|
get_mod_func should be last. |
chipx86
|
|
|
I think we can do one blank line before the TYPE_CHECKING block. Import-related conditionals are probably best considered as extra … |
chipx86
|
|
|
We now have a JSONDict type we can use here. |
chipx86
|
|
|
Same here. |
chipx86
|
|
|
We repeat this a lot. Worth defining an ExtensionMetadata type? |
chipx86
|
|
|
cast should be last (capitals are first). |
chipx86
|
|
|
These should be swapped. |
chipx86
|
|
|
**kwargs |
chipx86
|
|
|
These are missing docs. |
chipx86
|
|
|
While here, can we pull out registry so we don't re-fetch it every loop iteration? Same with __init__. |
chipx86
|
|
|
Can we set these types on the class? Same below. |
chipx86
|
|
|
multiple spaces before operator Column: 33 Error code: E221 |
reviewbot
|
|
|
'typing.Type' imported but unused Column: 1 Error code: F401 |
reviewbot
|
|
|
'typing.TYPE_CHECKING' imported but unused Column: 1 Error code: F401 |
reviewbot
|
|
|
line too long (87 > 79 characters) Column: 80 Error code: E501 |
reviewbot
|
|
|
line too long (82 > 79 characters) Column: 80 Error code: E501 |
reviewbot
|
|
|
Can you add a from __future__ import annotations here? |
chipx86
|
|
|
These get so lengthy and are common enough, I think we can just continue to fit as many as needed … |
chipx86
|
|
|
This line is too long. Wrapping before Type should work. |
chipx86
|
|
|
Missing docs. |
chipx86
|
|
|
This should probably be a JSONDict, since these will be stored in JSON. |
chipx86
|
|
|
Missing docs. |
chipx86
|
|
|
Missing Type:. |
chipx86
|
|
|
Thinking about what to do here, and came across this: https://stackoverflow.com/a/69418048 I think Option 2 makes a fair amount of … |
chipx86
|
|
|
Hmm wonder if we should just be storing this as a string when we assign it above. I think that … |
chipx86
|
|
|
Should have Type:. |
chipx86
|
|
|
Missing *args and **kwargs. |
chipx86
|
|
|
This can be super(). |
chipx86
|
|
|
Let's list public before private. Also, missing Type:. |
chipx86
|
|
|
Why this removal? |
chipx86
|
|
|
line too long (82 > 79 characters) Column: 80 Error code: E501 |
reviewbot
|
- Commits:
-
Summary ID 035171d8f2e6f3a9a38fb608513cb69a4116e1dd ab040ecec4168e740da8aeec189b389051d4ff22
Checks run (2 succeeded)
-
This is a great change! Thanks for tackling this.
-
Swap these lines.
Also, the
typingimports aren't alphabetized (TYPE_CHECKINGshould be beforeType). -
-
I think we can do one blank line before the
TYPE_CHECKINGblock. Import-related conditionals are probably best considered as extra import groups. -
-
-
-
-
-
-
-
While here, can we pull out
registryso we don't re-fetch it every loop iteration?Same with
__init__. -
- Commits:
-
Summary ID ab040ecec4168e740da8aeec189b389051d4ff22 84aa5ece1cf759b90104759029bc282bea1e4000
- Commits:
-
Summary ID 84aa5ece1cf759b90104759029bc282bea1e4000 24e0efc934880a86c8de65f2bdd1f58552c01c67
Checks run (2 succeeded)
- Change Summary:
-
Merge with latest changes for pkg_resources
- Commits:
-
Summary ID 24e0efc934880a86c8de65f2bdd1f58552c01c67 9cc989e6b5e4ba467d1b49503567a22371c8952e
- Commits:
-
Summary ID 9cc989e6b5e4ba467d1b49503567a22371c8952e 07aa2ae6bea336f22734d8258b8eaeb9ab32faff
Checks run (2 succeeded)
-
-
-
-
These get so lengthy and are common enough, I think we can just continue to fit as many as needed on one line.
-
-
-
-
Thinking about what to do here, and came across this:
https://stackoverflow.com/a/69418048
I think Option 2 makes a fair amount of sense in this particular case, since the signature is simple.
-
Hmm wonder if we should just be storing this as a string when we assign it above. I think that was the actual intent. Though that does technically break anything that might be using it as a UUID4.
-
-
-
-
-
- Description:
-
This isn't a complete typing for the extensions module, but it's enough
~ to quash a lot of the warnings that show up when working on exetnsions ~ to quash a lot of the warnings that show up when working on extensions code. - Commits:
-
Summary ID 07aa2ae6bea336f22734d8258b8eaeb9ab32faff 8a428af82d7691f210ddd061f54cfbbb97955860