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

Replace "Empty" with no-field output structs #518

Open
adamchalmers opened this issue Sep 17, 2024 · 0 comments · May be fixed by #520
Open

Replace "Empty" with no-field output structs #518

adamchalmers opened this issue Sep 17, 2024 · 0 comments · May be fixed by #520

Comments

@adamchalmers
Copy link
Collaborator

Right now, commands like Extrude that return no data will return the () or "unit". It's basically a struct with no fields. In the big enum of all response types, there's a variant Empty which corresponds to this.

But in the future, if we change this command to return some data, it'll be a breaking change because it changes the type of a response.

Suggested improvement: every modeling command should have an output struct, e.g. output::Extrude. If it has no data, that output struct will just have no fields. Then, if we add response data in the future, we can just add that field.

This will simplify the design of the crate because there'll be no need for a special Empty variant that works differently.

adamchalmers added a commit that referenced this issue Sep 17, 2024
@adamchalmers adamchalmers linked a pull request Sep 17, 2024 that will close this issue
adamchalmers added a commit that referenced this issue Sep 17, 2024
adamchalmers added a commit that referenced this issue Sep 17, 2024
adamchalmers added a commit that referenced this issue Sep 18, 2024
adamchalmers added a commit to KittyCAD/modeling-app that referenced this issue Sep 19, 2024
adamchalmers added a commit to KittyCAD/modeling-app that referenced this issue Sep 19, 2024
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 a pull request may close this issue.

1 participant