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 type stubs for lib.rs #14

Merged
merged 1 commit into from
Feb 12, 2024
Merged

Conversation

baranyildirim
Copy link
Contributor

@baranyildirim baranyildirim commented Feb 3, 2024

TLDR: Add type stubs for most of the code in lib.rs

Without type stubs, this library is quite hard to integrate into projects that have type-checking enabled.
Added type stubs based on the Rust types and documentation in lib.rs

Couldn't find an easy way to add types to Term (PyTerm). It is currently Any typed as a result.
All the code in biscuit_test.py now passes type checking with PyRight, so I assume the types are working correctly.

@divarvel
Copy link
Collaborator

divarvel commented Feb 3, 2024

Thanks, that's really valuable! Is there a way to add a relevant check in CI?

Could you drop the commit that reformats tests please? As long as there is no automatic formatting in this project I'd prefer PRs to be limited to actual changes.

(what formatter do you use? I would be open to a dedicated PR formatting the codebase and enforcing it in a dedicated CI step).

@baranyildirim
Copy link
Contributor Author

baranyildirim commented Feb 4, 2024

Thanks, that's really valuable! Is there a way to add a relevant check in CI?

There is but we would have to pick a type checker (pyre, mypy, pyright etc.) and add that to CI. Something like this: https://github.com/marketplace/actions/mypy-action

Could you drop the commit that reformats tests please? As long as there is no automatic formatting in this project I'd prefer PRs to be limited to actual changes.

Done!

(what formatter do you use? I would be open to a dedicated PR formatting the codebase and enforcing it in a dedicated CI step).

I was using Black.

@divarvel
Copy link
Collaborator

divarvel commented Feb 5, 2024

Alright, thanks! I guess we can add a new step in the existing github actions to run mypy on biscuit_test.py (I have tried quickly to trigger a type error but haven't succeeded)

@divarvel
Copy link
Collaborator

divarvel commented Feb 5, 2024

i will setup black in a follow up PR

@baranyildirim
Copy link
Contributor Author

Alright, thanks! I guess we can add a new step in the existing github actions to run mypy on biscuit_test.py (I have tried quickly to trigger a type error but haven't succeeded)

Added mypy action in the Build & Test workflow.

@divarvel divarvel merged commit 4747d1e into biscuit-auth:main Feb 12, 2024
1 check passed
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