Skip to content

Conversation

@cloutiertyler
Copy link
Contributor

@cloutiertyler cloutiertyler commented Nov 7, 2025

Description of Changes

This PR modifies the --delete-data flag on spacetime publish and adds the --delete-data flag on spacetime dev.

In particular instead of --delete-data being a boolean, it is now a an enum:

  • always -> corresponds to the old value of true
  • never -> corresponds to the old value of false
  • on-conflict -> clears the database, but only if publishing would have required a manual migration

This flag does NOT change any behavior about prompting users to confirm if they want to delete the data. Users will still be prompted to confirm UNLESS they pass the separate --yes flag.

spacetime dev gets the same --delete-data flag. The default value of never is equivalent to the existing behavior. spacetime dev continues to publish with --yes just as before. This behavior is unchanged.

API and ABI breaking changes

Adds the flags specified above. This is NOT a breaking change to the CLI. Passing --delete-data is the equivalent of --delete-data=always.

This IS technically a breaking change to the pre_publish route. As far as I'm aware this is only used by our CLI however.

IMPORTANT SIDE NOTE: I would argue that --break-clients should really be renamed to --yes-break-clients because it actually behaves like the --yes force flag, but only for a subset of the user prompts. I have not made this change because it would be a breaking change, but if the reviewers agree, I will make this change.

Expected complexity level and risk

2, Very small change, but if we get it wrong users could accidentally lose data. I would ask reviewers to think about ways that users might accidentally pass --delete-data --yes.

Testing

  • I have not yet tested manually.

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.

my code-owned changes LGTM, but left several small comments

@bfops bfops added the release-any To be landed in any release window label Nov 10, 2025
@cloutiertyler
Copy link
Contributor Author

Fixed all the comments!

@bfops
Copy link
Collaborator

bfops commented Nov 11, 2025

Smoketests failing due to:

Publishing module...
Updated database with name: etabxtydzldgaoqcnhcq, identity: c2004f38704bab11f9ba1c355adcdcd04f935040df8af34d65ede783b176c9ef
2025-11-11 18:48:39,464 - DEBUG - --------------

2025-11-11 18:48:39,464 - DEBUG - $ spacetime --config-path /tmp/tmpry9gvhzm/config.toml publish etabxtydzldgaoqcnhcq --project-path /tmp/tmpry9gvhzm --yes
2025-11-11 18:48:40,621 - DEBUG - --- stderr ---
   Compiling spacetime-module v0.1.0 (/tmp/tmpry9gvhzm)
    Finished `release` profile [optimized] target(s) in 0.32s
Error: Aborting because publishing would require manual migration or deletion of data and --delete-data was not specified.
2025-11-11 18:48:40,621 - DEBUG - --- stdout ---
Build finished successfully.
Uploading to localhost => http://127.0.0.1:3000/
Checking for breaking changes...
Errors occurred:
Adding a column age to table person requires a default value annotation

@bfops bfops disabled auto-merge November 12, 2025 17:43
@bfops
Copy link
Collaborator

bfops commented Nov 12, 2025

Disabling auto-merge since I was only approving my code-owned changes.

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.

Rest of the PR LGTM

@bfops bfops linked an issue Nov 12, 2025 that may be closed by this pull request
@bfops bfops removed a link to an issue Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-any To be landed in any release window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants