Skip to content

Conversation

@lumtis
Copy link
Contributor

@lumtis lumtis commented Feb 26, 2021

starport type foo bar ... --indexed creates a type that is indexed by a string. Methods to set and get this type in the store are scaffolded

Queries 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:

func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) {
	// this line is used by starport scaffolding # genesis/module/init
	for _, elem := range genState.FooList {
		k.SetFoo(ctx, *elem)
	}
	k.SetFoo(ctx, types.Foo{
		Index: "...",
		// ...
	})

}

@lumtis lumtis changed the title feat: Scaffold indexed type feat: scaffold indexed type Feb 26, 2021
@lumtis lumtis marked this pull request as ready for review February 26, 2021 14:21
@lumtis lumtis requested review from fadeev and ilgooz as code owners February 26, 2021 14:21
@lumtis lumtis linked an issue Feb 26, 2021 that may be closed by this pull request
Copy link
Contributor

@fadeev fadeev left a 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 { %>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
string index = 2;<%= for (i, field) in Fields { %>
string index = 1;<%= for (i, field) in Fields { %>

Not blocking.

Copy link
Member

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?

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

🍷

@ilgooz ilgooz merged commit ab3f4c7 into develop Feb 27, 2021
@ilgooz ilgooz deleted the feat/indexed-type branch February 27, 2021 07:11
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

command to create an indexed type

4 participants