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

Implemented two endpoints in the client: puzzles.next() and studies.get_from_user() #85

Closed

Conversation

ShuvraneelMitra
Copy link

@ShuvraneelMitra ShuvraneelMitra commented Dec 6, 2024

Checklist when adding a new endpoint
  • Added new endpoint to the README.md
  • Ensure that my endpoint name does not repeat the name of the client. Wrong: client.users.get_user(), Correct: client.users.get()
  • Typed the returned JSON using TypedDicts in berserk/types/, example
  • Tested GET endpoints not requiring authentification. Documentation, example
  • Added the endpoint and your name to CHANGELOG.md in the To be released section (to be created if necessary)

@ShuvraneelMitra ShuvraneelMitra changed the title Implemented two endpoints in the client Implemented two endpoints in the client: puzzles.next() and studies.get_from_user() Dec 6, 2024
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! Thanks for the PR, we are on the right track, couple comments left here and there.

The returned responses are not typed though, it'd be nice to have them. In the checklist for a PR you can see an example.

You have ticked the item about tests (nice!) but I don't see them, maybe you have not committed the file?

@ShuvraneelMitra
Copy link
Author

ShuvraneelMitra commented Dec 6, 2024

Hi @kraktus , thanks for your feedback. I'll be honest, when I saw "test GET ..." I just thought I had to check the results on a python environment, not create full testing scripts! On the other hand, I've never written tests for anything before, so I'll give myself a pass on that lol.

Coming to more pressing issues:
I have created the testing for the methods not requiring authentication for the puzzles and studies modules. However I run into an error while submitting the PR which seems to stem from the fact that someone hardcoded some asserts in another module which I didn't open. Can you look into it?

@ShuvraneelMitra ShuvraneelMitra closed this by deleting the head repository Feb 5, 2025
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.

2 participants