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

quickstart: Build Soroban RPC From New soroban-rpc Repo #558

Closed
wants to merge 7 commits into from

Conversation

stellarsaur
Copy link

Part of CLI/RPC repo split. We should build from soroban-rpc now.

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.

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

Copy link
Member

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, less code, it's only used for ad-hoc make build which implies specifying versions for components in any case, @Shaptic and @psheth9, should we move this pr along still?

Copy link

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)

Copy link
Contributor

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.

@psheth9 psheth9 self-assigned this Feb 21, 2024
Copy link

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.

@github-actions github-actions bot added the stale label Mar 23, 2024
@mollykarcher mollykarcher deleted the rpc-repo-split branch March 23, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants