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

fix: options provisioning #272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fredmaggiowski
Copy link
Member

@fredmaggiowski fredmaggiowski commented Sep 7, 2022

I noticed that the builder for the direct service proxy API is provided with the baseOptions parameter while the correct parameter should be options. Diving a bit into the past merge requests I found this one where the provided param was options.

I believe that #80 introduced a bug, even though current tests are still passing.

Checklist

  • your branch will not cause merge conflict with master
  • run npm test
  • tests are included
  • the documentation is updated or integrated accordingly with your changes
  • the code will follow the lint rules and style of the project
  • you are not committing extraneous files or sensible data

@coveralls
Copy link

coveralls commented Sep 7, 2022

Coverage Status

Coverage remained the same at 97.627% when pulling e8ab83b on fix/options-provisioning into a044368 on master.

@davidebianchi
Copy link
Member

Great catch! It seems correct the fix, can you add a test to verify it works?

@fredmaggiowski
Copy link
Member Author

fredmaggiowski commented Sep 7, 2022

Great catch! It seems correct the fix, can you add a test to verify it works?

I honestly don't have much time right now 😓 I'm trying to add a test later

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.

4 participants