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.

Sign up for GitHub

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

Jump to bottom

Remove unnecessary dependencies #149

Merged
merged 1 commit into from
Apr 21, 2024
Merged

Remove unnecessary dependencies #149

merged 1 commit into from
Apr 21, 2024

Conversation

2opremio
Copy link
Contributor

@2opremio 2opremio commented Apr 21, 2024 •
edited
Loading

No description provided.

Shaptic
Shaptic approved these changes Apr 21, 2024
View reviewed changes
Copy link
Contributor

@Shaptic Shaptic left a comment

Choose a reason for hiding this comment

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

🔥

@2opremio 2opremio merged commit 738a3e4 into main Apr 21, 2024
20 checks passed
@2opremio 2opremio deleted the remove-deps branch April 21, 2024 20:07
leighmcculloch
leighmcculloch reviewed Apr 21, 2024
View reviewed changes
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.

👏🏻 Nice. One comment inline about a couple other deps I think we can remove, and now would be a good time because of the nature of the change being breaking.

Cargo.toml
Comment on lines 29 to 30
# rev = "c30bc769e379bef9b94a3ceb464aa78c1185eeb3"
# path = "../rs-soroban-sdk/soroban-spec"
Copy link
Member

Choose a reason for hiding this comment

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

I think we can also remove soroban-spec as a dependency and soroban-sdk further down, which would then mean that soroban-rpc then has zero dependency on the stellar/rs-soroban-sdk repository.

The stellar-rpc-client uses those two crates to lookup the wasm specs from a .wasm file it downloads from the soroban-rpc. Imo it's outside the responsibility of the stellar-rpc-client to do the decoding of the wasm. The client should help an application download the .wasm, and then leave it up to the application to extract the spec. Applications that want to extract the spec, like the soroban-cli, can import soroban-spec and soroban-sdk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@willemneal could you push out the wasm spec lookup to soroban-cli?

@willemneal willemneal mentioned this pull request Apr 23, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leighmcculloch leighmcculloch leighmcculloch left review comments

@Shaptic Shaptic Shaptic approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@2opremio @leighmcculloch @Shaptic