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 Python bindings generation command #1761

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

overcat
Copy link
Contributor

@overcat overcat commented Nov 27, 2024

What

See #1633 (comment)

Why

If this link can be provided in the CLI, it may be easier for users to find it; otherwise, they might not know that Python bindings can be generated.

Known limitations

N/A

Copy link
Member

@leighmcculloch leighmcculloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One tweak requested, see the inline comment.

I think we should merge this. Today the cli is where folks go to generate bindings, and so it makes sense as part of that story to support discoverability of a commonly used sdk.


We haven't started discussions about this yet, but when I saw that you implemented the Python SDK's codegen tool outside the CLI, it prompted me to start thinking about doing the same for the JS SDK. I think the pattern you've used is a good pattern to follow for the following reason. These codegen tools are generating code for using the SDKs, and so they are tightly coupled with the SDKs, and specific versions of the SDKs. We've already found it difficult to coordinate stellar-cli releases with the js-stellar-sdk, and if we moved the JS/TS binding generation from being "cli adjacent" to being "sdk adjacent" then I think it would tell a better maintainer story.

The thing I'm wrestling with is discoverability, the same issue for why this PR exists.

If we do this we wouldn't likely do it before v23. I just wanted to drop some thoughts because I think the setup we have today with bindings built into the cli may not be the setup in the near future, and so this area of code and functionality may change.

cmd/soroban-cli/src/commands/contract/bindings/python.rs Outdated Show resolved Hide resolved
@overcat
Copy link
Contributor Author

overcat commented Nov 27, 2024

Hi @leighmcculloch, I completely agree with your point of view. Another reason I didn't implement it directly in this repository is that, for me personally, developing generation scripts using a scripting language may be more efficient than using Rust. I can write and debug at the same time without having to wait for long compilation times. 😂

@leighmcculloch leighmcculloch merged commit a11a924 into stellar:main Nov 27, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants