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.

Sign up for GitHub

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

Jump to bottom

Fix @name not working on single elem group choices #213

Merged
merged 2 commits into from
Nov 21, 2023
Merged

Fix @name not working on single elem group choices #213

merged 2 commits into from
Nov 21, 2023

Conversation

rooooooooob
Copy link
Collaborator

@rooooooooob rooooooooob commented Nov 15, 2023 •
edited
Loading

Fixes #211
Fixes #153

Also adds in usage of @name into existing test cases.

@rooooooooob rooooooooob requested a review from gostkin November 15, 2023 22:32
SebastienGllmt
SebastienGllmt approved these changes Nov 15, 2023
View reviewed changes
Copy link
Collaborator

@SebastienGllmt SebastienGllmt left a comment

Choose a reason for hiding this comment

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

code looks good, but maybe we want to add tests to cover these cases to avoid regressions?

@rooooooooob
Copy link
Collaborator Author

I added in uses of @name into a few of the existing test cases. These cover group choices (including the case addressed in this PR), type choices and map fields. We didn't test @name anywhere before this. You can only test that the tests compile so maybe that's why we didn't have any in the first place since it was easier to see if it worked or not compared to runtime testable things like deserialization accuracy. Either way it was very easy to put in some so why not.

@rooooooooob rooooooooob merged commit d84f78a into master Nov 21, 2023
1 check passed
@SebastienGllmt SebastienGllmt deleted the name-fix-single-elem-variant branch December 25, 2023 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SebastienGllmt SebastienGllmt SebastienGllmt approved these changes

@gostkin gostkin Awaiting requested review from gostkin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

@name doesn't work for single fixed element group choice @name not applied with single-field group choices
2 participants
@rooooooooob @SebastienGllmt