- 
                Notifications
    You must be signed in to change notification settings 
- Fork 570
feat: scaffold indexed type #790
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
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.
Code generation looks good! 👍
| import "gogoproto/gogo.proto"; | ||
|  | ||
| message <%= title(TypeName) %> { | ||
| string index = 2;<%= for (i, field) in Fields { %> | 
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.
| string index = 2;<%= for (i, field) in Fields { %> | |
| string index = 1;<%= for (i, field) in Fields { %> | 
Not blocking.
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.
We don't have a creator field here anymore, is it fine to not have it?
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.
Also, what if instead of creating a new message type, can't we have an option in the existent one that enables indexing with the user given string instead? By default indexing is disabled.
This feels like will simplify our code generation logic and can be more obvious for the user.
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 now the type is only managed internally, you may have no creator, it's up to the developer to decide. For example, in the IBC exchange, you will have an OrderBook which is indexed by PortID*ChannelId*TokenPair you don't have creator for this one.
Also, what if instead of creating a new message type, can't we have an option in the existent one that enables indexing with the user given string instead? By default indexing is disabled.
Indexed type respond to a different logic, methods are a bit different than the other type. And you have less method generated. Also we don't know how those two kind of type will evolve in the future. We need a different template for better maintainability.
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.
🍷
* Rename placeholder vars * Refactoir add type options * Template * Scaffold file modification * Implement get all * Indexed flag * Integration tests * Fix bugds * Lint * Fix getAll issue Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
starport type foo bar ... --indexedcreates a type that is indexed by a string. Methods to set and get this type in the store are scaffoldedQueries to get the type instances are created:
list-foo,show-foo <index>The type can only be created internally. Messages to set and remove instances will be implemented in another PR.
Since there are no messages to add types, the app can be tested by adding types in
InitGenesis: