-
Couldn't load subscription status.
- Fork 570
feat: support multiple name convention for component and field names #1236
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
Conversation
|
When it's ready for review, this should work, right? Or is it too much? 🙈 |
|
Yes. Not in this example because only the message name is parsed |
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.
Looks very good!
starport/pkg/field/field_test.go
Outdated
| }, | ||
| }, | ||
| } | ||
| cases.expected[0].Name, _ = multiformatname.NewMultiFormatName("foo") |
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.
can we refactor this to execute each test over a loop like a table test?
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.
I made this part a bit cleaner but I'm not sure it it what you meant. This is already a table test but we have a single test that match an expected with an actual array
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.
Brilliant! One question I have that may be better addressed in a different PR is, don't we accept numbers in field names? Right now it's telling me "name cannot contain 5", but I'm not sure why.
No I can actually add this, we just need to prevent having a number as a prefix |
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.
🔥
For names (component or field) we will support only alpha characters, hyphen or underscore. This is open for discussion.
Maybe we could prevent using underscore since snake case is never used in the code but we should support hyphen since this is more convient for message and query scaffolding
@fadeev @ilgooz I did the implementation for the
messagescaffold as an example. I will wait for your feedback before moving forward