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

Fix order attribute not being mandatory in some situations #2618

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Jul 6, 2023

Fixes #2527


Checklist

@teh-cmc teh-cmc added 🪳 bug Something isn't working codegen/idl labels Jul 6, 2023
@teh-cmc teh-cmc force-pushed the cmc/order_mandatory branch 2 times, most recently from 639b717 to 8ef3849 Compare July 6, 2023 13:20
Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Why is it always mandatory, even when there is only one field in a struct? It adds so much noise :/

@teh-cmc
Copy link
Member Author

teh-cmc commented Jul 7, 2023

Why is it always mandatory, even when there is only one field in a struct? It adds so much noise :/

I'm all for adding user-friendly exceptions such as this yep.
For now I just wanted to fix this bug because order is veeeeeeeeery important when enums/unions become involved!

@teh-cmc teh-cmc force-pushed the cmc/order_mandatory branch from 8ef3849 to 525a3e7 Compare July 7, 2023 14:24
@teh-cmc teh-cmc merged commit a6568a1 into main Jul 7, 2023
@teh-cmc teh-cmc deleted the cmc/order_mandatory branch July 7, 2023 14:25
@teh-cmc
Copy link
Member Author

teh-cmc commented Jul 7, 2023

teh-cmc added a commit that referenced this pull request Jul 7, 2023
…2626)

**COMMIT BY COMMIT!**

What the title says.

Adding support for sparse unions would be trivial, but nobody ever uses
those so I've refrained from doing it for now.

Fixes #2625 
Requires #2618 
Requires #2619

---

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2626) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2626)
- [Docs preview](https://rerun.io/preview/pr%3Acmc%2Farrow_union/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Acmc%2Farrow_union/examples)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working codegen/idl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

order should be mandatory but it doesn't seem to be
2 participants