Added support for getting leaderboard size back with position #4433
Conversation
There was a problem hiding this comment.
Just a quick initial review from my phone, also I noticed something like 578 states the return type in an int, that's incorrect since it's an Optional[int], would you mind tweaking that while working on this pr? If not it's fine it can be done in a separate PR.
You will notice I've renamed some of the variables you have added, this is to ensure we are following PEP and keeping variable naming motion consistent across the project.
Jackenmen
added
Category: Core - API - Bank
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
Co-authored-by: Draper <[email protected]>
|
Great feedback. I have a better idea on contributing to this project now! |
Added Tuple import from typing
Ran make reformat to fix style
|
I think all of the requested changes are done. Let me know if not. Thanks! |
There was a problem hiding this comment.
This is awkward, I forgot to assign it to me so I missed your changed, Approving for now.
@jack1142 this one is on the milestone for 3.4.11, but just in case merging would make your life harder, please go ahead and merge when it is a good time.
| @@ -559,27 +559,36 @@ async def get_leaderboard(positions: int = None, guild: discord.Guild = None) -> | |||
|
|
|||
|
|
|||
| async def get_leaderboard_position( | |||
| member: Union[discord.User, discord.Member] | |||
| ) -> Union[int, None]: | |||
| member: Union[discord.User, discord.Member], return_lb_size: bool = False | |||
There was a problem hiding this comment.
| member: Union[discord.User, discord.Member], return_lb_size: bool = False | |
| member: Union[discord.User, discord.Member], *, return_lb_size: bool = False |
Jack suggested this should be changed into a keyword-only arg, so if you can do this too it would be awesome.
There was a problem hiding this comment.
Once the pending suggestion is addressed I can merge this one for you @MrBillson
Type
Description of the changes
Added support for getting leaderboard size back with position. You can now pass in an optional param that is a flag to return the overall leaderboard size or not. This is the enhancement requested in issue 4200. This is a backwards compatible non-breaking change.
closes #4200