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

Finish spacetimedb-update functionality #2126

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

coolreader18
Copy link
Collaborator

@coolreader18 coolreader18 commented Jan 14, 2025

Description of Changes

based off #2011

Adds implementations to all the subcommands of spacetime version.

Expected github release artifact structure:

  • artifact filename: spacetime-{rustc-target-triple}.{ext}
    • ext = if windows { "zip" } else { "tar.gz" }
  • artifact contents:
    • spacetimedb-cli[.exe], spacetimedb-standalone[.exe], spacetimedb-update[.exe]

cc @clockworklabs/devops - feedback on this^^ would be nice. I kept the edge cases since that's how it already was and I figured that might make it easier in the release workflow, but honestly I'd prefer if it was just spacetime-{rustc-target-triple}.{ext} if possible. Not sure where the release flow is or if it's even automated. Implemented in #2193.

Expected complexity level and risk

3 - a complex bit of code that's implementing a theoretically perpetual contract between spacetimedb-update, github release artifacts, and the directory structure.

Testing

  • Manually tested that spacetime version install works
  • I'm working on an automated regrtest to make sure the functionality works.
  • reviewer: if you wanna, play around with it and try and catch out any edge cases?

@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch 5 times, most recently from 621fba2 to 59ec2a0 Compare January 17, 2025 18:17
@coolreader18 coolreader18 marked this pull request as ready for review January 17, 2025 20:43
@bfops
Copy link
Collaborator

bfops commented Jan 18, 2025

(It looks like this currently isn't passing CI, so I'm holding off on review)

@bfops
Copy link
Collaborator

bfops commented Jan 29, 2025

Review notes:

  • The previous functionality of spacetime version is now accomplished by spacetime --version (and spacetime version list indicates which version is the current one)

@bfops
Copy link
Collaborator

bfops commented Jan 29, 2025

Note to self: I still need to:

  • review crates/cli/src/subcommands/version.rs
  • review crates/paths
  • review crates/update itself
  • jot down what I think the surface area for testing is here

@coolreader18 coolreader18 mentioned this pull request Jan 29, 2025
1 task
@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch from 35e5488 to 030c9b3 Compare January 31, 2025 18:50
@coolreader18
Copy link
Collaborator Author

I removed the rm-core-as-dep-of-cli commit from this branch, I'll do it as a follow-up PR.

@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch from 030c9b3 to 80940cc Compare January 31, 2025 19:00
@bfops
Copy link
Collaborator

bfops commented Jan 31, 2025

(I merged in master to bump CI. The Internal tests were failing due to them running before https://github.com/clockworklabs/SpacetimeDBPrivate/pull/1279 merged).

@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch 2 times, most recently from 05186de to 7f72ea7 Compare February 4, 2025 22:22
@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch from 7f72ea7 to e296ddb Compare February 5, 2025 19:41
Copy link
Collaborator

@bfops bfops left a comment

Choose a reason for hiding this comment

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

After the conversation elsewhere today, I'm approving this as-is. If we need to do further testing, changes, or code cleanups, we will have to do them in followup PRs. For now let's merge this and start getting it tested more widely.

@bfops
Copy link
Collaborator

bfops commented Feb 5, 2025

(That said, CI seems to be failing)

@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch from e296ddb to d6c9122 Compare February 6, 2025 19:53
@coolreader18 coolreader18 force-pushed the noa/spacetime-update-impl branch from d6c9122 to 68f3581 Compare February 6, 2025 20:10
@coolreader18 coolreader18 added this pull request to the merge queue Feb 6, 2025
Merged via the queue into master with commit e6775eb Feb 6, 2025
12 of 13 checks passed
@coolreader18 coolreader18 deleted the noa/spacetime-update-impl branch February 11, 2025 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Directory structure - Implement new spacetimedb-update cli
2 participants