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

Update client.js to add RPC suffixes #271

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

kimveasna
Copy link
Contributor

@kimveasna kimveasna commented Mar 9, 2020

Description

Additional RPC suffixes are appended to manage SOAP RFC from SAP auto-generated webservices.

Related issues

Checklist

  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style
    guide

@kimveasna kimveasna changed the title Update client.js to consider additional RPC suffixes Update client.js to add RPC suffixes Mar 10, 2020
@kimveasna kimveasna marked this pull request as ready for review March 10, 2020 10:00
@kimveasna kimveasna force-pushed the master branch 3 times, most recently from f6af2eb to 570e4c7 Compare March 16, 2020 09:50
@emonddr
Copy link

emonddr commented Mar 23, 2020

@kimveasna , thank you for the fix.

@@ -333,7 +333,7 @@ class Client extends Base {
result = obj.Body[outputName];
}
if (!result) {
Copy link
Contributor

Choose a reason for hiding this comment

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

some how there are two if blocks implementing the same condition

https://github.com/strongloop/strong-soap/blob/92239c41e2135eeab3c3ae0ec2e4f46253558945/src/client.js#L330-L334

https://github.com/strongloop/strong-soap/blob/92239c41e2135eeab3c3ae0ec2e4f46253558945/src/client.js#L335-L341

along with the changes in this PR we could fix the first block with the regex as well
(?:(Out|\.Out)(?:put)?|(Response|\.Response))$

Copy link
Contributor

@deepakrkris deepakrkris left a comment

Choose a reason for hiding this comment

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

noticed two if blocks handling similar if conditions, the first one with regex could also be improved.
I am approving this PR anyway.
LGTM

@@ -333,7 +333,7 @@ class Client extends Base {
result = obj.Body[outputName];
Copy link
Contributor

@deepakrkris deepakrkris Mar 23, 2020

Choose a reason for hiding this comment

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

we can fix the previous regex block as well :
(?:(Out|\.Out)(?:put)?|(Response|\.Response))$

@emonddr
Copy link

emonddr commented Mar 24, 2020

@kimveasna , please squash your commits.
https://loopback.io/doc/en/lb4/submitting_a_pr.html#10-final-rebase-and-squashing-of-commits

e.g.

pick ...  
fixup ...

@kimveasna
Copy link
Contributor Author

@kimveasna , please squash your commits.
https://loopback.io/doc/en/lb4/submitting_a_pr.html#10-final-rebase-and-squashing-of-commits

e.g.

pick ...  
fixup ...

Thank you for the instructions. All done.

@raymondfeng raymondfeng merged commit 8eb43b3 into loopbackio:master Mar 25, 2020
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