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

Support version on waitForServices #112

Merged
merged 3 commits into from Oct 1, 2017
Merged

Conversation

imatefx
Copy link
Contributor

@imatefx imatefx commented Sep 30, 2017

If a service has version waitForServices() function was not able to find the service because the version is undefined when passed to registry.hasService()

it checks if the version can be extracted from the service names with format

v1.accounts
v2.users

@coveralls
Copy link

coveralls commented Sep 30, 2017

Coverage Status

Coverage decreased (-0.04%) to 98.541% when pulling 81de8c6 on imatefx:develop into 0eadc11 on ice-services:master.

@icebob
Copy link
Member

icebob commented Oct 1, 2017

Thanks, but current solution works only with numeric version.
We should support all options, e.g.:

  • v3.users
  • staging.users
  • v2.order.saga
    ...etc

Would be better to set an object if you want to define version.

Normal current format

waitForServices("users");
waitForServices(["posts", "users"]);

New optional versioned format

waitForServices({ name: "users", version: "staging" });
waitForServices([{ name: "order.saga", version: 2 }, {name: "users", version: "staging"]);

Both format should be supported.

And of course please add relevant test cases too. We make an effort to cover all codes to ensure stability & quality codebase.

Could you change it?

@imatefx
Copy link
Contributor Author

imatefx commented Oct 1, 2017

ok, i will change it

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0006%) to 98.585% when pulling 17db54f on imatefx:develop into 0eadc11 on ice-services:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0006%) to 98.585% when pulling 17db54f on imatefx:develop into 0eadc11 on ice-services:master.

@coveralls
Copy link

coveralls commented Oct 1, 2017

Coverage Status

Coverage increased (+0.0006%) to 98.585% when pulling 2da950c on imatefx:develop into 0eadc11 on ice-services:master.

@imatefx
Copy link
Contributor Author

imatefx commented Oct 1, 2017

I saw your edit only few minutes back 😄 , Added tests

Copy link
Member

@icebob icebob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect PR! Thanks!

@icebob icebob merged commit d6a9297 into moleculerjs:master Oct 1, 2017
@icebob
Copy link
Member

icebob commented Oct 1, 2017

Nice job! Thank you! 👍

@imatefx
Copy link
Contributor Author

imatefx commented Oct 1, 2017

😄 Thats my contribution to Open Source after a long time 👍

@imatefx imatefx deleted the develop branch October 1, 2017 15:57
@icebob
Copy link
Member

icebob commented Oct 6, 2017

Released in v0.11.2

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

Successfully merging this pull request may close these issues.

None yet

3 participants