- Notifications
You must be signed in to change notification settings - Fork673
feat(api)!: Make RESTObjectList typing generic#3121
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
I made this simple python file to check type hints: importtypingfromgitlab.v4.objectsimportMergeRequestManagerdeftest_list(mang:MergeRequestManager)->None:forminmang.list():typing.reveal_type(m) When run on main branch the revealed type will be: Using this branch the revealed type will be: I believe this is because both |
codecovbot commentedFeb 6, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@## main #3121 +/- ##======================================= Coverage 97.28% 97.28% ======================================= Files 97 97 Lines 5975 5976 +1 =======================================+ Hits 5813 5814 +1 Misses 162 162
Flags with carried forward coverage won't be shown.Click here to find out more.
|
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.
Thanks@igorp-collabora again! This LGTM.
Could you maybe add a breaking change footer as outlined inhttps://www.conventionalcommits.org/en/v1.0.0/, with a short sentence outlying the return types for list endpoints will now be the real object rather than RESTObject?
This way it should be easier for downstream users to migrate to our new major bump (I'll look at other deprecations in our code and maybe add more breaking changes as well this month).
77cb2e9 toa24c2cfCompareBREAKING CHANGE: Type narrowing of `list()` methods return objectsfrom RESTObject to a concrete subclass (for example `MergeRequest`)can become redundant.Currently the RESTObjectList type hints yielded objectsas base RESTObject. However, the ListMixin is now genericand will return the RESTObject subclass based on the RESTManagertyping.Using `typing.Generic` it is possible to make RESTObjectListtype hint a specific subclass of the RESTObject.Iterating over `list()` call the ListMixin will now yieldthe same object class because both `list` and `RESTObjectList`will have the same type hinted subclass.Signed-off-by: Igor Ponomarev <igor.ponomarev@collabora.com>
a24c2cf tod290a21CompareI added the breaking changes footer to the commit message and an exclamation mark to the commit summary. |
befba35 intopython-gitlab:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
BREAKING CHANGE: Type narrowing of
list()methods return can become redundant.Currently the RESTObjectList type hints yielded objects as base RESTObject. However, the ListMixin is now generic and will return the RESTObject subclass based on the RESTManager typing.
Using
typing.Genericit is possible to make RESTObjectList type hint a specific subclass of the RESTObject.Iterating over
list()call the ListMixin will now yield the same object class because bothlistandRESTObjectListwill have the same type hinted subclass.Closes#2062