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

feat: expose Rust API #9

Merged
merged 10 commits into from
Dec 5, 2024
Merged

feat: expose Rust API #9

merged 10 commits into from
Dec 5, 2024

Conversation

denehoffman
Copy link
Contributor

Addresses #6

Here's a quick PR that should allow for the same Python API to be accessible via Rust via the crate prelude. All I did was expose the required structs and dupe the method that validates email. It was already written "the correct way", throwing a Rust Result and translating it to a PyResult, all I did was split the method so users could get the Rust Result independently.

I have not (yet) tested this API beyond the existing test suite, and I didn't include a distribution GitHub action workflow, although I can add one if you'd like. I would recommend release-plz. Let me know if you'd like me to update the PR with that workflow file. I can't test that part myself since the repo owner should also be the crate owner, so you'll have to go on crates.io and get an API code and add it to the repo secrets.

Let me know what you think and if you'd like anything else added.

@bnkc
Copy link
Owner

bnkc commented Dec 3, 2024

@denehoffman I like the initial approach here. Yes feel free to add a workflow file. If at all possible, please add some comments within the struct for the end user to know what each attribute is, as well as updating the README.md 🎉

@denehoffman
Copy link
Contributor Author

While I was doing that, I'm realizing that something is a bit wonky with the python implementation itself, have you tested a fresh install recently? I'm getting circular import errors when I try import emval. I think I can fix this real quick, I'll push something to this PR in a bit.

@denehoffman
Copy link
Contributor Author

While I was doing that, I'm realizing that something is a bit wonky with the python implementation itself, have you tested a fresh install recently? I'm getting circular import errors when I try import emval. I think I can fix this real quick, I'll push something to this PR in a bit.

I spoke too soon, it works fine, something was messed up on my end.

@denehoffman
Copy link
Contributor Author

You'll need to follow the instructions at https://release-plz.ieni.dev/docs/github/quickstart to set up the cargo registry token and GitHub token, I can't do that from my end.

@denehoffman
Copy link
Contributor Author

Before you do this, I need to add the required fields to Cargo.toml, just realized this.

@denehoffman
Copy link
Contributor Author

The only thing left is how you want to link to the crates.io / docs.rs / pypi pages, if at all. docs.rs gets automatically populated when you upload to crates.io, so you might want to a link or some shields (https://shields.io/).

@bnkc
Copy link
Owner

bnkc commented Dec 3, 2024

@denehoffman Good work! I will try to get this merged by EOW 🤞🏻

@denehoffman
Copy link
Contributor Author

@bnkc Glad to help! I saw your reddit post by the way, that's how I ended up here.

@bnkc bnkc merged commit 49df9e9 into bnkc:main Dec 5, 2024
15 of 16 checks 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