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

Add an OpenAPI #7

Merged
merged 3 commits into from
Jan 9, 2022
Merged

Add an OpenAPI #7

merged 3 commits into from
Jan 9, 2022

Conversation

Seon82
Copy link
Contributor

@Seon82 Seon82 commented Jan 7, 2022

Closes #6
Websocket specification is currently not supported, see this issue.

@BlankParenthesis
Copy link
Owner

This looks pretty good overall. I noticed a few potential issues however:

  • The URI for the default board is incorrect — it should be /boards/default rather than /board/default.
  • POST for placing a pixel lacks a request body — it should be JSON with the single key "color".
  • The MinimalBoard representation is missing the max_pixels_available property.

These should be fairly trivial to fix and I'd be happy to merge this once they are and add support for more extensions later.

One final thing that I noticed: ValidationError and the corresponding HTTPValidationError don't seem to be used anywhere and aren't part of the spec as far as I know. Perhaps there's some reason for their existence related to OpenAPI but otherwise I think I'd prefer to see those removed since they seem like either a mistake or noise to me.

@BlankParenthesis BlankParenthesis merged commit 5e34a3f into BlankParenthesis:master Jan 9, 2022
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.

Use OpenAPI
2 participants