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

server.method breaking change #1935

Closed
dhm116 opened this issue Sep 11, 2014 · 2 comments
Closed

server.method breaking change #1935

dhm116 opened this issue Sep 11, 2014 · 2 comments
Assignees
Labels
breaking changes Change that can breaking existing code non issue Issue is not a problem or requires changes
Milestone

Comments

@dhm116
Copy link

dhm116 commented Sep 11, 2014

It appears there is a breaking change to how methods added via server.method or plugin.method are handled between v6.7.1 and 6.8.0 - v6.7.1...v6.8.0#diff-89cf897642f2ca735f4178aa8760dec6R1282

It appears as though registered methods need to accept and invoke a callback now. I would consider this a breaking change, especially since the methods we were registering were using promises and now requires un-wrapping the promise to invoke the callback and wrapping the callback results in a promise.

I suppose this relates to #1930 in that it wasn't apparent this behavior had changed.

@hueniverse hueniverse added breaking changes Change that can breaking existing code non issue Issue is not a problem or requires changes labels Sep 11, 2014
@hueniverse hueniverse added this to the 6.8.0 milestone Sep 11, 2014
@hueniverse hueniverse self-assigned this Sep 11, 2014
@hueniverse
Copy link
Contributor

It was a bug of omission. The documents always made it clear that methods must use callbacks. Also, there really is no point in using server methods if you are not going to use them with the callback because you might as well just use good old plain node modules to share them as utilities.

Because you were not using it as specified, I don't consider this a change that merits a major version bump, but I did flag it as breaking change and linked to the version so it will show up in the lists.

@dhm116
Copy link
Author

dhm116 commented Sep 12, 2014

Cool, thanks for the clarification 👍

@lock lock bot locked as resolved and limited conversation to collaborators Jan 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code non issue Issue is not a problem or requires changes
Projects
None yet
Development

No branches or pull requests

2 participants