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 #37

Merged
merged 2 commits into from
Apr 17, 2023
Merged

add type stubs #37

merged 2 commits into from
Apr 17, 2023

Conversation

maxbachmann
Copy link
Contributor

@maxbachmann maxbachmann commented Sep 16, 2022

@gfairchild this is how I implement installation + type stubs in my projects. This avoids the duplication from #34.

The only duplicated file is the actual type hint, which can't be avoided when working with c extensions.

@gfairchild
Copy link
Collaborator

Nice! At first glance, this is pretty much what I was hoping for with no duplication.

@gfairchild
Copy link
Collaborator

Do you know how we can test this? It'd be great to incorporate something into the current unit-testing framework.

@maxbachmann
Copy link
Contributor Author

I have the following test in my projects:
https://github.com/maxbachmann/RapidFuzz/blob/f69be93ca84e28354d16c874cb0b2d6407f238cd/.github/workflows/branchbuild.yml#L117-L120

@gfairchild
Copy link
Collaborator

@maxbachmann
Copy link
Contributor Author

@gfairchild are there any plans to geht this merged? Otherwise I will close the PR.

@gfairchild
Copy link
Collaborator

I apologize. I'd love to spend more time reviewing this, but I just haven't had time. I don't really use this code base much anymore.

I'd prefer to leave the PR open instead of close.

@maxbachmann
Copy link
Contributor Author

Unless I have a special interest in a PR, I close them at some point around the 1 year mark. However feel free to merge this into a branch and create your own PR you can look into if you ever find the time.

@gfairchild
Copy link
Collaborator

I'm going to go ahead and merge. Thank you, @maxbachmann, for your contribution, and I apologize for how long it took me to act on this.

@gfairchild gfairchild merged commit b6e085b into lanl:master Apr 17, 2023
This was referenced Apr 17, 2023
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