Skip to content

Conversation

@mamcx
Copy link
Contributor

@mamcx mamcx commented Nov 27, 2025

Description of Changes

Closes #3673

NOTE: C++ part will be in another PR

Expected complexity level and risk

2

Adding a new type touch everywhere

Testing

  • Adding smoke and unit test

@mamcx mamcx self-assigned this Nov 27, 2025
@mamcx mamcx added the release-any To be landed in any release window label Nov 27, 2025
@mamcx mamcx force-pushed the mamcx/result-type branch from 70ca89a to 4324cd7 Compare November 27, 2025 18:50
@mamcx mamcx requested a review from gefjon November 27, 2025 19:01
@JasonAtClockwork
Copy link
Contributor

I'll build out an attached PR to add the new functionality for the Unreal SDK.

@mamcx mamcx force-pushed the mamcx/result-type branch 3 times, most recently from 5a99c91 to cf9dd2f Compare December 15, 2025 15:20
@mamcx mamcx marked this pull request as ready for review December 15, 2025 16:50
Copy link
Contributor

@JasonAtClockwork JasonAtClockwork left a comment

Choose a reason for hiding this comment

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

There is updates to the /modules/sdk-test-cs but this won't be enough testing for the C# side. We'll need to add tests into the /sdks/csharp/examples~/regression-tests like for UUID to make sure this works end-to-end.

@rekhoff - I'll also want your eyes on this one when you can for the C# side

Copy link
Contributor

@gefjon gefjon left a comment

Choose a reason for hiding this comment

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

I'm happy with the Rust parts of this PR. We'll still need review from the other languages' maintainers.

Copy link
Contributor

@rekhoff rekhoff left a comment

Choose a reason for hiding this comment

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

The C# side looks good. I did some local testing of end-to-end behavior, and things look good.

Copy link
Collaborator

@coolreader18 coolreader18 left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, except for my one comment.

@mamcx mamcx force-pushed the mamcx/result-type branch 3 times, most recently from 604c452 to c7b1f53 Compare December 29, 2025 20:52
@mamcx mamcx force-pushed the mamcx/result-type branch 2 times, most recently from 32ebe84 to bd8dba7 Compare December 31, 2025 15:22
@mamcx mamcx force-pushed the mamcx/result-type branch from bd8dba7 to d5bd9e0 Compare December 31, 2025 15:23
@JasonAtClockwork JasonAtClockwork self-requested a review December 31, 2025 20:21
Copy link
Contributor

@JasonAtClockwork JasonAtClockwork left a comment

Choose a reason for hiding this comment

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

I've added a couple comments but otherwise this looks like a solid fix for tests on the C# side!

@mamcx mamcx force-pushed the mamcx/result-type branch from a85626f to 370e005 Compare December 31, 2025 20:52
@mamcx mamcx force-pushed the mamcx/result-type branch from 370e005 to 6898dc4 Compare December 31, 2025 21:29
@rekhoff rekhoff added this pull request to the merge queue Jan 8, 2026
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2026
# Description of Changes

Closes #3673 

*NOTE*: C++ part will be in another PR

# Expected complexity level and risk

2

Adding a new type touch everywhere

# Testing

- [x] Adding smoke and unit test

---------

Signed-off-by: Ryan <r.ekhoff@clockworklabs.io>
Co-authored-by: rekhoff <r.ekhoff@clockworklabs.io>
Co-authored-by: Phoebe Goldman <phoebe@goldman-tribe.org>
Co-authored-by: Jason Larabie <jason@clockworklabs.io>
Merged via the queue into master with commit 82d5a4f Jan 8, 2026
73 of 79 checks passed
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.

Implement SpacetimeType for Result<T, E>

6 participants