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.

Sign up for GitHub

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

Jump to bottom

baseapp: NewBaseApp takes a Codec #1441

Closed
ebuchman opened this issue Jun 28, 2018 · 2 comments
Closed

baseapp: NewBaseApp takes a Codec #1441

ebuchman opened this issue Jun 28, 2018 · 2 comments

Comments

@ebuchman
Copy link
Member

This is awkward in the documentation since we have to pass in a codec but then might overwrite it with SetTxDecoder. I think we should either pass in a TxDecoder and remove SetTxDecode, or not pass in anything and just use SetTxDecoder (and maybe expose a AminoTxDecoder(cdc *wire.Codec) TxDecoder
@rigelrozanski
Copy link
Contributor

+1 for removing SetTxDecoder, when would we ever even want to modify the decoder after we've created the baseapp anyways?

@alexanderbez
Copy link
Contributor

Seems that cdc codec is used in BaseApp#Query. Apart from that, it makes sense to have the constructor take a TxDecoder 👍

@ValarDragon ValarDragon mentioned this issue Jul 19, 2018
Closed
22 tasks
ValarDragon added a commit that referenced this issue Sep 4, 2018
@ValarDragon
baseapp: Remove baseapp.SetTxDecoder()
ea01b91 
Closes #1441
@ValarDragon ValarDragon mentioned this issue Sep 4, 2018
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ebuchman @alexanderbez @rigelrozanski