- Notifications
You must be signed in to change notification settings - Fork673
feat(client): introduceiterator=True
and deprecateas_list=False
inlist()
#2032
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
7825972
toc678721
Comparec678721
tob4ae128
Comparecodecov-commenter commentedMay 29, 2022
Codecov Report
@@ Coverage Diff @@## main #2032 +/- ##==========================================- Coverage 92.72% 92.71% -0.02%========================================== Files 78 78 Lines 4947 4953 +6 ==========================================+ Hits 4587 4592 +5- Misses 360 361 +1
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.
Looks good@JohnVillalovos just a few minor comments.
Context for future readers:
https://github.com/python-gitlab/python-gitlab/pull/1956/files#r884302316
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
… in `list()``as_list=False` is confusing as it doesn't explain what is beingreturned. Replace it with `iterator=True` which more clearly explainsto the user that an iterator/generator will be returned.This maintains backward compatibility with `as_list` but does issue aDeprecationWarning if `as_list` is set.
b4ae128
tocdc6605
Comparelist()
changeas_list=False
toiterator=True
iterator=True
and deprecateas_list=False
inlist()
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@JohnVillalovos, makes sense to me, also makes it consistent with the iterator arg on the upcoming streaming responses PR then.
Uh oh!
There was an error while loading.Please reload this page.
as_list=False
is confusing as it doesn't explain what is beingreturned. Replace it with
iterator=True
which more clearly explainsto the user that an iterator/generator will be returned.
This maintains backward compatibility with
as_list
but does issue aDeprecationWarning if
as_list
is set.