Skip to content

fix: add serviceName, methodName, methodIndex to rpcImpl (#1229)#2145

Open
lili2012 wants to merge 2 commits intoprotobufjs:masterfrom
lili2012:rpcImpl
Open

fix: add serviceName, methodName, methodIndex to rpcImpl (#1229)#2145
lili2012 wants to merge 2 commits intoprotobufjs:masterfrom
lili2012:rpcImpl

Conversation

@lili2012
Copy link
Copy Markdown

@lili2012
Copy link
Copy Markdown
Author

Unfortunately, as written, the code breaks all current users of rpcImpl because it changes the signature of the function they are supposed to implement. So, whenever this change is released, everyone who uses the current rpcImpl will need to go update their code. It might be possible to add new optional parameters to the existing functions, but you must not reorder parameters or introduce new parameters in the middle.

I put serviceName, methodName, methodIndex at the end of rpcImpl function parameters. and no breaks to any users of rpcImpl. Hope you could review it and accept my pull request. Thanks!

@alexander-fenster

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.

1 participant