-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
Patch 1 #263
Patch 1 #263
Conversation
Merge suggestion by @pavel-kirienko Co-authored-by: Pavel Kirienko <pavel.kirienko@gmail.com>
This change fixes the support for a built-in, default variable length array implementation for C++ types that can be overridden by the user. To accomplish this the concept of “support resource type” was added to allow different behavior for serialization support resource than for datatype support resources.
almost there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding comments to guide the reviewer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good, not a C++ guru so I'm not sure I can provide appropriate feedback to the internal_compare function of the VLA.
Python code gen sections look clean. I'm not familiar with the documentation format of the doc strings, in particular lines 134-154 of src/nunavut/lang/cpp/init.py just looks like left over code to me.
There is one potentially serious issue discovered by Sonar though: https://sonarcloud.io/project/issues?pullRequest=263&issues=AYKyzbSpjX57U6EJSHck&open=AYKyzbSpjX57U6EJSHck&id=OpenCyphal_nunavut |
Kudos, SonarCloud Quality Gate passed! |
As promised, completed work to support the built-in variable_length_array built-in.
I still owe this project: