-
Notifications
You must be signed in to change notification settings - Fork 206
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
quickstart: Build Soroban RPC From New soroban-rpc Repo #558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I think we need to merge this after the first release of soroban-rpc on the new repo though, unless we're backfilling all the tags on the soroban-rpc repo? Which we could do.
@@ -16,9 +16,9 @@ fi | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm inclined to say we should remove this script instead of keep it. In all cases where we specify a version of rpc we also specify a version explicitly of horizon. This script is only used by the makefile, but it should be easier now-days to pick versions of horizon that match versions of soroban-rpc. Wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont have enough context on script removal part but this PR is pointing to old version of rpc, latest released soroban-rpc version is 20.3.2
so we need to update that. overall yes, we should definitely update quickstart so that it points to new soroban-rpc. Are there any edge cases here? cc @mollykarcher (if not I can refactor this PR and land)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can follow your suggestion and be landed with a little more adjustment like you mentioned for 20.3.2, regarding build-future.yml
, that refers to futurenet, and I don't think that network has been updated to core 20.2.0 , which would mean leave it's soroban-tools/tag as-is, unless the same older tag is copied on soroban-rpc repo, the futurenet network option probably needs to be deprecated from quickstart on a separate ticket.
can sanity test pr locally by running the make targets for latest
or testing
image variants, and run the image and watch console output to confirm it joins the network:
docker run --rm -it -p "8000:8000" --name stellar stellar/quickstart:latest|testing --testnet
can use --local
for network also on either of those image variants, quickstart will run a standalone network in the container using CORE_REF version, rather than connecting to remote.
This pull request is stale because it has been open for 30 days with no activity. It will be closed in 30 days unless the stale label is removed. |
Part of CLI/RPC repo split. We should build from
soroban-rpc
now.