Skip to content

Fix order attribute not being mandatory in some situations#2618

Merged
teh-cmc merged 3 commits intomainfrom
cmc/order_mandatory
Jul 7, 2023
Merged

Fix order attribute not being mandatory in some situations#2618
teh-cmc merged 3 commits intomainfrom
cmc/order_mandatory

Conversation

@teh-cmc
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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