Skip to content

Conversation

@zach-klippenstein
Copy link
Collaborator

Value classes get boxed as soon as they're used as a supertype, so it's basically pointless to make sealed class children value classes. I made it a data class instead.

This change is required to move :workflow-runtime-android into :workflow-runtime, or we get a compiler crash.

I also made the object siblings data objects to fix a compiler suggestion and get better toString methods.

Value classes get boxed as soon as they're used as a supertype, so it's basically pointless to make sealed class children value classes. I made it a data class instead.

This change is required to move `:workflow-runtime-android` into `:workflow-runtime`, or we get a compiler crash.

I also made the object siblings data objects to fix a compiler suggestion and get better `toString` methods.
@zach-klippenstein zach-klippenstein requested a review from a team as a code owner July 16, 2025 23:21
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* the runtime produces a rendering.
*/
public object RenderPassSkipped : RuntimeUpdate
public data object RenderPassSkipped : RuntimeUpdate
Copy link
Collaborator

Choose a reason for hiding this comment

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

We avoid using data in public API, remember? Seems like this would be fine as just object / class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The arguments for not using data classes in public API don't apply to objects, since they contain no data by definition.

And the data class was previously a value class, which is even more impossible to change than a data class, so this is no more constrained.

@zach-klippenstein zach-klippenstein merged commit 286060d into main Jul 17, 2025
64 checks passed
@zach-klippenstein zach-klippenstein deleted the zachklipp/remove-valueclass branch July 17, 2025 00:00
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