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 issue when searching for 1.2 SOAP operations #26

Closed
wants to merge 1 commit into from

Conversation

andrewhr
Copy link

This code was raising exceptions about not having a variable declared in case of 1.2 operations.

I've wanted to add at least an integration test for this fix, but didn't find any WSDL to break the actual code. And unfortunately, the WSDL used to discover the issue was a proprietary service from a current client, so I'm not allowed to share 😢

This code was raising exceptions about not having a variable declared
in the case of 1.2 operations.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling dface3f on andrewhr:master into 6e3ad7e on savonrb:master.

@rubiii
Copy link
Contributor

rubiii commented May 11, 2013

thanks @andrewhr. the code is obviously wrong. not sure why it wasn't covered with tests, since the fixtures actually include an example for a soap 1.2 service, but if you look at the changelog, the next release will probably be a major release containing the rewrite. so that piece of code is already gone.

@rubiii rubiii closed this May 11, 2013
@andrewhr
Copy link
Author

Great! But the library rewrite is not available on current master? Because is from the master that I catch this error.

I've been using Savon for a while and the gem is awesome! Thank you for the work, you rock! 🎉 🎊

Can I help you with something to release this improvements and fixes? I will love to rely on the WSDL for the services I need to consume (actually, the current release is not able to handle then).

@rubiii
Copy link
Contributor

rubiii commented May 11, 2013

thanks 😊

the new code is on wasabi master. it's not integrated with savon yet.
there's still a lot of things to do (see #27), but i'll let you know when this can be tested through savon.

@andrewhr
Copy link
Author

Ok! I'll visit the issues later and see if I can pick something. (at least make the build green on jruby!)

@rubiii
Copy link
Contributor

rubiii commented May 11, 2013

@andrewhr that would be awesome. i started investigating it yesterday but wasn't able to reduce the testcase to something i could show to the nokogiri guys. it looks like a difference between the c- and java-versions, but for some reason it worked fine in a simple testcase.

@rubiii
Copy link
Contributor

rubiii commented May 12, 2013

@andrewhr i was looking at the wrong element. i opened a bug at sparklemotion/nokogiri#902.

@andrewhr
Copy link
Author

I was investing yesterday but didn't write an example to show to nokogiri's team (in fact, I never did something like that before).

So I just add a comment about my investigation to your open issue. Hope it helps!

@rubiii
Copy link
Contributor

rubiii commented May 12, 2013

@andrewhr great. thank you very much!

@rubiii
Copy link
Contributor

rubiii commented May 12, 2013

@andrewhr if you think you can replace the wsdl with a more simple example, please feel free to open a pull request.
it probably helps debugging the problem.

@andrewhr
Copy link
Author

@rubiii I will do it now! 👌

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

Successfully merging this pull request may close these issues.

3 participants