-
Notifications
You must be signed in to change notification settings - Fork 85
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
feat: support listing all branches #178
Conversation
*/ | ||
@Deprecated |
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.
Hi and thank you for your contribution! I don't think we really need to deprecate this method?
Did a similar PR recently: #176 without deprecating the non paged method. Otherwise I think this PR looks good!
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.
Somehow I think developers should be warned to not use the old method and point them in a better direction. Depreciation was the clearest way I could think of for doing that. But I agree that depreciation is a bit wrong in the sense that the old method isn't likely to be removed.
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.
As the public methods in this repo usually have javadocs, I think it might be nice to just mention the new (better) method in the javadocs without labelling the old method as deprecated..
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 good think with deprecation is that it gives a visible warning in peoples IDEs. But OK, how about this?
Also added convenience method for making a stream of paginated results
1febf45
to
b9d9f14
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #178 +/- ##
============================================
- Coverage 77.23% 77.17% -0.07%
- Complexity 308 311 +3
============================================
Files 43 44 +1
Lines 1028 1034 +6
Branches 44 44
============================================
+ Hits 794 798 +4
- Misses 209 211 +2
Partials 25 25 ☔ View full report in Codecov by Sentry. |
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.
This now sounds like a nice compromise to hint lib users to use the paged method instead!
Also added convenience method for making a stream of paginated results