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

Added ability to return an error for failed SecuritySchema checker. #625

Merged
merged 7 commits into from
Aug 10, 2023

Conversation

NexRX
Copy link
Contributor

@NexRX NexRX commented Aug 8, 2023

Hi, I thought I'd give a crack at adding support for returning errors for the Security Schema checker.

I've never writen a proc macro before so let me know if there is a better way to implictly take a Option or Result without knowing what you're getting at compile time (I really tried to find out :()

Thanks!

@sunli829
Copy link
Collaborator

sunli829 commented Aug 9, 2023

Looks great, can you add a test case to here? 🙂

@NexRX
Copy link
Contributor Author

NexRX commented Aug 9, 2023

Pushed those tests 🤖. Was waiting for feedback before commiting. Also updated security scheme markup doc ✨

@sunli829
Copy link
Collaborator

sunli829 commented Aug 9, 2023

good job, thank you! ❤️

@sunli829
Copy link
Collaborator

sunli829 commented Aug 9, 2023

format code with cargo +nightly fmt

@NexRX
Copy link
Contributor Author

NexRX commented Aug 9, 2023

Is the pipeline failing because of me? Because running cargo +nightly fmt locally I can see 261 changes when i only chnaged 3 files. Ill push it anyway, let me know if you want it reset.

Done. Had some quirks with git status :D

@sunli829
Copy link
Collaborator

sunli829 commented Aug 9, 2023

We should move CheckerReturn type to poem-openapi crate and expose it in poem::__private, otherwise there may be an error of duplicate type definition.

This problem should not exist, but in order to reduce compilation time, it is still hoped that the CheckerReturn type can be moved to poem-openapi. 🙂

@NexRX
Copy link
Contributor Author

NexRX commented Aug 9, 2023

We should move CheckerReturn type to poem-openapi crate and expose it in poem::__private, otherwise there may be an error of duplicate type definition.

This problem should not exist, but in order to reduce compilation time, it is still hoped that the CheckerReturn type can be moved to poem-openapi. 🙂

It does feel silly having it there, I initially put it there just to see how to implicitly get multiple return types working, but ill move that now.

@sunli829 sunli829 merged commit dccbd34 into poem-web:master Aug 10, 2023
@NexRX
Copy link
Contributor Author

NexRX commented Aug 10, 2023

Awsome! ✨

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