RSDK-9370: change param prefix check in module generation #4654
+61
−57
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
stuqdog
reviewed
Dec 23, 2024
There was a problem hiding this comment.
Abstractly this code all looks reasonable to me, but it's a lot of indexing math and string manipulation that I don't really have context for. It would be great if we could add a test somewhere to show that we're now correctly generating type names for maps, pointers, channels, etc., or at a minimum add a screenshot showing the corrected behavior (as opposed to the behavior in the screenshot on the ticket).
| typeString := buf.String() | ||
|
|
||
| prefixes := []string{"*", "[]*", "[]", "chan "} | ||
| // checkUpper attributes to <resourceSubtype> if type is capitalized after prefix. |
There was a problem hiding this comment.
(minor) I think there's a word missing between "checkUpper" and "attributes"? Either that or i'm just having trouble parsing the grammar here.
viambot
added
safe to test
stuqdog
approved these changes
Jan 2, 2025
viambot
added
safe to test
purplenicole730
deleted the
RSDK-9370-expand-param-prefix-check-in-module-generation
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ticket found a bug where pointer param types weren't attributing to the resource subtype.
Return and param types were treated differently and separately, so even though return types were attributing pointer types correctly, params were not. Now, types are being formatted the same in both returns and types. Instead of a switch statement, a for loop that contains prefix types (
"*", "[]*", "[]", "chan ") was introduced for scalability and readability.Testing:
formatTypearm,board, andcameracomponents andvision service. Not sure if there's other resources with more kinds of types that should be tested.arm component:
board component:
vision service:
