Skip to content

[Rust] Allow to supply a default string value - #5562

Open
kistz wants to merge 25 commits into
clockworklabs:bfops/consolidate-default-testsfrom
kistz:deafult-string-fix
Open

[Rust] Allow to supply a default string value#5562
kistz wants to merge 25 commits into
clockworklabs:bfops/consolidate-default-testsfrom
kistz:deafult-string-fix

Conversation

@kistz

@kistz kistz commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description of Changes

Rust modules cannot do it at the moment.
Allow it by taking in a &'static str.
The approach in the macro is safe because we always have a std::String enforeced by the SpacetimeType implementation.

API and ABI breaking changes

None. Additive changes

TODO: Update the documentation aswell if approach is good enough.

Expected complexity level and risk

1

Testing

  • Migrated a test module locally and it worked
  • Added a test for it

@kistz

kistz commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

@gefjon could you take a look at this? :>
if the approach is sufficiently good i would adjust the docs 😇

@bfops bfops mentioned this pull request Jul 29, 2026
2 tasks

@gefjon gefjon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems totally reasonable, though it will need updates to be compatible with #5630 , and to use the tests in #5618 . I am unsure whether we'll merge this PR or that chain first.

@bfops

bfops commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Regardless of the order of PR merges, I think the tests in #5618 are the best coverage for this, so I had the bot revert the test changes. Other than that, I'm fine to merge in any order if you want to move this out of draft.

@bfops
bfops changed the base branch from master to bfops/consolidate-default-tests July 31, 2026 08:01
@kistz

kistz commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

@bfops @gefjon cool ty :>
i adjsuted the docs (removed rust disclaimer)

moved out of draft and ready to merge from my side :>

Have a good one guys!

( if i am required to do anything else np just tell me 👍 )

@kistz
kistz marked this pull request as ready for review July 31, 2026 12:02
@bfops
bfops requested a review from gefjon July 31, 2026 12:23
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.

4 participants