Skip to content
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

Ensure action tags are added for all msg types #1406

Closed
cwgoes opened this issue Jun 27, 2018 · 5 comments
Closed

Ensure action tags are added for all msg types #1406

cwgoes opened this issue Jun 27, 2018 · 5 comments
Labels

Comments

@cwgoes
Copy link
Contributor

cwgoes commented Jun 27, 2018

This allows users to search for particular types of transactions. Action tags don't yet exist in x/bank, and may also need to be added elsewhere.

cc @gamarin2

@gamarin2
Copy link
Contributor

We need this for launch, at least for bank module @cwgoes

@mossid
Copy link
Contributor

mossid commented Jul 4, 2018

To avoid adding action tags manually, we can add result.Tags = append(result.Tags, sdk.NewTag("action", reflect.TypeOf(msg).Name())) in runTx

@rigelrozanski
Copy link
Contributor

Meh not too big of a fan of defining Tags in multiple locations, I'd personally like to see them consolidated, but doesn't super matter

@ValarDragon
Copy link
Contributor

ValarDragon commented Aug 26, 2018

result.Tags = append(result.Tags, sdk.NewTag("action", reflect.TypeOf(msg).Name()))

alternatively, I think adding a name function to msgs is reasonable. The purpose would be just to avoid reflection here. With reflection, any name change for the message struct is a state breaking change.

I like the idea of avoiding adding action tags manually.

@alexanderbez
Copy link
Contributor

++ @ValarDragon & @rigelrozanski

ValarDragon added a commit that referenced this issue Sep 15, 2018
This is to facillitate ease of implementing #1406. (Tags for messages
could then be added dynamically)

Ultimately once we make the router support hiearchical routing, (#770)
we can then remove the name field and just the parse info for tags from that.

Until then, we can parse the tag name as
`fmt.Sprintf("%s %s", msg.Type(), msg.Name())`
cwgoes pushed a commit that referenced this issue Sep 17, 2018
This is to facillitate ease of implementing #1406. (Tags for messages
could then be added dynamically)

Ultimately once we make the router support hiearchical routing, (#770)
we can then remove the name field and just the parse info for tags from that.

Until then, we can parse the tag name as
`fmt.Sprintf("%s %s", msg.Type(), msg.Name())`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants