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

Rename Msg.Name() to Msg.MsgType() and Msg.Type() to Msg.MsgRoute() #2456

Closed
sunnya97 opened this issue Oct 9, 2018 · 7 comments · Fixed by #2553
Closed

Rename Msg.Name() to Msg.MsgType() and Msg.Type() to Msg.MsgRoute() #2456

sunnya97 opened this issue Oct 9, 2018 · 7 comments · Fixed by #2553

Comments

@sunnya97
Copy link
Member

sunnya97 commented Oct 9, 2018

Can we rename the method .Name() on the Msg interface? The field Name is something that many modules may want to use and it can be hard to come up with a good synonym for it. i.e. this broke my nameservice module

@ValarDragon
Copy link
Contributor

I propose changing the Name method to Type and the Type method to Route

@sunnya97
Copy link
Member Author

sunnya97 commented Oct 9, 2018

I proposer Name -> MsgType and Type -> MsgRoute

@cwgoes
Copy link
Contributor

cwgoes commented Oct 11, 2018

I proposer Name -> MsgType and Type -> MsgRoute

Three thumbs up, seems past the threshold of "proposal-accepted" - sounds good to me!

@cwgoes cwgoes changed the title Rename Msg.Name() Rename Msg.Name() to Msg.MsgType() and Msg.Type() to Msg.MsgRoute() Oct 11, 2018
@fedekunze
Copy link
Collaborator

fedekunze commented Oct 11, 2018

Msg.MsgType() and Msg.MsgRoute() seem redundant to me. Why not just Msg.Type() and Msg.Route() ?

@ValarDragon
Copy link
Contributor

ValarDragon commented Oct 11, 2018

I agree with @fedekunze. I don't think we should prepend msg either. I think it is reasonable to reserve Type and Route as methods on message types. I highly doubt that it will inconvenience that many people, whereas MsgType and MsgRoute does seem confusing.

@mossid
Copy link
Contributor

mossid commented Oct 11, 2018

If we had .Name() it would make the struct unable to contain Name field. Since Name is likely to be used in various situations, it makes sense to rename it to MsgName(). However we already have type keyword in golang and the name Route will not be used widely like as Name, I think we can assume both will not be used in the structs, so it is safe to rename it to Type() and Route().

@cwgoes
Copy link
Contributor

cwgoes commented Oct 16, 2018

I'm fine with .Type() and .Route() too, either option is better that what we have at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants