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

Add extension group support #33

Merged

Conversation

matzipan
Copy link
Contributor

@matzipan matzipan commented Dec 23, 2023

Related to #3

  • Add test

Still missing "all" and "choice" (#20).

@matzipan
Copy link
Contributor Author

matzipan commented Dec 23, 2023

I don't really like this implementation. A nicer one would get the fields for the target struct and create a list instead of just a reference (same for the extension base). But I wasn't sure how to reach that so if you have any suggestions, it's welcome. Maybe a two-pass implementation?

@matzipan
Copy link
Contributor Author

matzipan commented Jan 2, 2024

@MarcAntoine-Arnaud do you think there's any path to get this merged? If not, could you please comment on what adjustments I should make.

@MarcAntoine-Arnaud MarcAntoine-Arnaud merged commit 214ab9b into media-io:main Jan 2, 2024
@MarcAntoine-Arnaud
Copy link
Contributor

Sorry more time was needed to understand your code.
I have review it and merged.

Thank you !

@matzipan
Copy link
Contributor Author

matzipan commented Jan 2, 2024

Sorry my comment was not meant as pressure - I just wanted to get a rough idea.

@matzipan matzipan deleted the implement_group_extension branch January 2, 2024 21:04
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.

2 participants