Skip to content
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

implement api/player/autocomplete #48

Merged

Conversation

handsamtw
Copy link
Contributor

Implementing api/player/autocomplete api.
I got one questions regarding the param "friend", the name and definition of this parameter is unclear to me.
Thus, I didn't add it to the implementation at this moment. Hope that somebody could explain to me.

The Documentation link: https://lichess.org/api#tag/Users/operation/apiPlayerAutocomplete

@handsamtw handsamtw changed the title implement api/player/autocomplete implement api/player/autocomplete #6 Oct 16, 2023
@handsamtw handsamtw changed the title implement api/player/autocomplete #6 implement (#6) api/player/autocomplete Oct 16, 2023
@handsamtw
Copy link
Contributor Author

Work on #6

@handsamtw handsamtw changed the title implement (#6) api/player/autocomplete implement api/player/autocomplete Oct 16, 2023
Copy link
Member

@kraktus kraktus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thank you very much for the PR! I’ve left a few comments quickly, these just be seen as instructive feedback. Since it’s late I may be terse in my wordings so please ask if something is unclear to you.

If you do not have the time to fix those, I’ll eventually pick it up. Cheers

berserk/clients/users.py Outdated Show resolved Hide resolved
berserk/clients/users.py Outdated Show resolved Hide resolved
berserk/clients/users.py Outdated Show resolved Hide resolved
berserk/clients/users.py Outdated Show resolved Hide resolved
@handsamtw
Copy link
Contributor Author

Hello @kraktus , I have made all necessary changes to resolve the comment you left previously.
However, when I tried make format operation locally, this error happen, and my format pipeline also failed.
Not sure this is something I can fix from my side. Appreciate your time reviewing.

Screenshot 2023-10-20 at 11 27 24 AM

kraktus added a commit to kraktus/lila that referenced this pull request Oct 21, 2023
…characters

All client-code for Lichess already check that and lichobile does too: https://github.com/lichess-org/lichobile/blob/663e69fab10e4267a9b3369febe85d1363816ba2/src/ui/players/PlayersCtrl.ts#L51

prompted by lichess-org/berserk#48.

Note that currently it does sometimes return a result (undocumented), hitting the `userIdsLikeCache` cache, so maybe this is intended.
kraktus added a commit to kraktus/lila that referenced this pull request Oct 21, 2023
…characters

All client-code for Lichess already check that and lichobile does too: https://github.com/lichess-org/lichobile/blob/663e69fab10e4267a9b3369febe85d1363816ba2/src/ui/players/PlayersCtrl.ts#L51

prompted by lichess-org/berserk#48.

Note that currently it does sometimes return a result (undocumented), hitting the `userIdsLikeCache` cache, so maybe this is intended.
@handsamtw
Copy link
Contributor Author

Hello @kraktus , are we good to merge and closed this pr?

@kraktus
Copy link
Member

kraktus commented Oct 23, 2023

Hey, thanks for the good work, I'd still like to get a test to ensure the lightuser type is correct, and add the follower parameter (can take it from #53)

I'll finish in a few days when I'm more available.

@kraktus kraktus merged commit 8b8b8b0 into lichess-org:master Oct 28, 2023
21 checks passed
@kraktus
Copy link
Member

kraktus commented Oct 28, 2023

Thanks! Implementing a test allowed to find several issues: e6cc202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants