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

Rename soroban-rpc to stellar-rpc #644

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

2opremio
Copy link
Contributor

No description provided.

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.

Couple minor things. It also looks like builds are failing so maybe something else in there needs tweaking to fix that.

The builds are failing with:

ERROR: failed to solve: failed to read dockerfile: open cmd/stellar-rpc/docker/Dockerfile: no such file or directory

.github/workflows/build.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@2opremio
Copy link
Contributor Author

@leighmcculloch PTAL

Makefile Show resolved Hide resolved
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 request.

stellar_rpc_ref: v22.1.0
stellar_rpc_ref: fb9f8c7c146485e0d4f7e4285e62e1678c89ce3d
Copy link
Member

Choose a reason for hiding this comment

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

Can a version of stellar-rpc be released for quickstart to reference?

While it functionally works to ref the git commit, that's intended to be used for dev and testing or in PRs. When users look at what version is in use, it won't report a released version. The testnet tag is reserved for rc and stable releases, and the latest tag is reserved for stable releases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's worth going through the hassle of releasing just for this. It will get updated to a stable tag in the next release.

Copy link
Member

Choose a reason for hiding this comment

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

If it's not worth releasing rpc with the new name, whats the motivation for breaking out of the regular process, and releasing a special unreleased version into QuickStart now rather than updating it at the rpc next stable release?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because QuickStart will be broken without the update and we need it for system tests and finish the renaming. See stellar/system-test#107

Copy link
Contributor

Choose a reason for hiding this comment

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

I would agree that quickstart should only have released versions referenced here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CI is broken in stellar-rpc because of this

Copy link
Member

@leighmcculloch leighmcculloch Nov 26, 2024

Choose a reason for hiding this comment

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

The CI that's broken, can you update it to point to this PR? This PR is published to: stellar/quickstart:pr644-v459-latest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I will give that a try

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you take a look at stellar/system-test#107 ?

@2opremio 2opremio enabled auto-merge (squash) November 26, 2024 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog (Not Ready)
Development

Successfully merging this pull request may close these issues.

4 participants