Skip to content

Conversation

@kim
Copy link
Contributor

@kim kim commented Aug 23, 2023

Description of Changes

I have a bunch of toolchains installed, so unconditionally running rustup update can take a while. We only need the active one to be in sync with rust-toolchain.toml, though.

API and ABI

  • This is a breaking change to the module ABI
  • This is a breaking change to the module API
  • This is a breaking change to the ClientAPI
  • This is a breaking change to the SDK API

If the API is breaking, please state below what will break

@kim kim requested a review from jdetter August 23, 2023 08:52
@kim kim requested a review from coolreader18 September 6, 2023 10:30
Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Seems to work for me locally, testsuite is passing, LGTM 👍

@kim kim enabled auto-merge (squash) September 7, 2023 19:22
@kim kim merged commit c010bfa into master Sep 7, 2023
bfops added a commit that referenced this pull request Jul 17, 2025
## Description of Changes
Remove the `-hotfix*` part of the version in our trunk branch.

It was a (my) mistake to merge a `-hotfix*` version change. Hotfixes
are, by definition, cherry-picked changes that are going to be released
directly to users without merging.

The consequence of merging this change was that the `SDK Tests` CI job
started failing on SpacetimeDB PRs (e.g. see failures on
#2137), because it
checks out this repo, which then tried to use the `-hotifx3` version of
SpacetimeDB. But the `master` branch of SpacetimeDB is at `1.0.0-rc3`
(no hotfix), because the hotfix was correctly released from a branch
without merging in that repo.

Although this PR reverts the version change, we do still have a tag
`v1.0.0-rc3-hotfix3` that we can use to release the hotfix version (by
`git push -f origin v1.0.0-rc3-hotfix3:master`) if/when desired.

## API

 - [ ] This is an API breaking change to the SDK

No

## Requires SpacetimeDB PRs
Should work with `master`.

## Testsuite
*If you would like to run the your SDK changes in this PR against a
specific SpacetimeDB branch, specify that here. This can be a branch
name or a link to a PR.*

SpacetimeDB branch name: master

## Testing
I claim that the CI tests passing with `master` show that this is
correct. The original version change itself passed CI because it was
pointing at a non-`master` branch for testing. I claim it would have
failed if it were tested against SpacetimeDB `master`.

Generally, this might point to a bug in how we've done this CI: We
should probably only allow a PR to merge in this repo if it tests
successfully against SpacetimeDB `master`, even if we want to point it
at other branches to test in the meantime.

Co-authored-by: Zeke Foppa <[email protected]>
bfops added a commit that referenced this pull request Aug 7, 2025
## Description of Changes
Remove the `-hotfix*` part of the version in our trunk branch.

It was a (my) mistake to merge a `-hotfix*` version change. Hotfixes
are, by definition, cherry-picked changes that are going to be released
directly to users without merging.

The consequence of merging this change was that the `SDK Tests` CI job
started failing on SpacetimeDB PRs (e.g. see failures on
#2137), because it
checks out this repo, which then tried to use the `-hotifx3` version of
SpacetimeDB. But the `master` branch of SpacetimeDB is at `1.0.0-rc3`
(no hotfix), because the hotfix was correctly released from a branch
without merging in that repo.

Although this PR reverts the version change, we do still have a tag
`v1.0.0-rc3-hotfix3` that we can use to release the hotfix version (by
`git push -f origin v1.0.0-rc3-hotfix3:master`) if/when desired.

## API

 - [ ] This is an API breaking change to the SDK

No

## Requires SpacetimeDB PRs
Should work with `master`.

## Testsuite
*If you would like to run the your SDK changes in this PR against a
specific SpacetimeDB branch, specify that here. This can be a branch
name or a link to a PR.*

SpacetimeDB branch name: master

## Testing
I claim that the CI tests passing with `master` show that this is
correct. The original version change itself passed CI because it was
pointing at a non-`master` branch for testing. I claim it would have
failed if it were tested against SpacetimeDB `master`.

Generally, this might point to a bug in how we've done this CI: We
should probably only allow a PR to merge in this repo if it tests
successfully against SpacetimeDB `master`, even if we want to point it
at other branches to test in the meantime.

Co-authored-by: Zeke Foppa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants