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

stubs for xtea library #9740

Closed
wants to merge 7 commits into from
Closed

stubs for xtea library #9740

wants to merge 7 commits into from

Conversation

rajohnson
Copy link

Adds stubs for the xtea encyption library.

I think that this is ready to review, unless CI catches something I've missed.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Jelle Zijlstra <[email protected]>
@hauntsaninja
Copy link
Collaborator

Note that stub_uploader is different from stubtest.

stub_uploader will not be happy with pep272_encryption as a requirement because of:
https://github.com/typeshed-internal/stub_uploader/blob/32dca63e0cdb13393454bde4ed76fefa72d99a4f/stub_uploader/metadata.py#L151-L163
I should write up an issue that's not on that PR, but see the link in code

@github-actions

This comment has been minimized.

@rajohnson
Copy link
Author

@hauntsaninja My understanding is that pep272_encryption would have to be reviewed by the typeshed team and manually added to EXTERNAL_REQ_ALLOWLIST in stub_uploader/metadata.py in order for this to pass. Is this correct?

pep272_encryption is pretty obscure and was created by the author of the library that I'm trying to make stubs for. I'm not sure that it will be worth your time if my understanding is correct, is it worth continuing this pull request?

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@hauntsaninja
Copy link
Collaborator

Yeah, I think it would be a little challenging to add pep272_encryption to that allowlist. I wrote up typeshed-internal/stub_uploader#90 which has details of the exact situation.

Three alternative options are:

  • Given that the author of xtea has type annotations on pep272_encryption, maybe they'd be willing to accept a PR adding annotations directly to xtea? In general, if types can be upstreamed that's preferable to having types in typeshed
  • I'm not sure that this would work well here, but we could just include the API of PEP272Cipher in XTEACipher and have users of these stubs rely on substructural typing (Protocols)
  • If you're willing to wait, maybe we can resolve some of the barriers that make us hesitant to add things to the allowlist in stub_uploader, as described in that issue

In all cases, we're very happy to review the stubs themselves and are grateful for your contribution of them! :-)

@rajohnson
Copy link
Author

rajohnson commented Feb 17, 2023

Ok, contributing the stubs to xtea makes a lot of sense, I'll pursue that option first. Thanks for your help!

@rajohnson rajohnson closed this Feb 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.

3 participants