search

savonrb / wasabi

A simple WSDL parser
MIT License
90 stars 84 forks source link

BC BREAKING: Revert "Add support for rpc encoded wsdl" #103

Closed fsateler closed 3 years ago

fsateler commented 3 years ago

This change breaks backwards compatibility, by changing the parsing of the SOAP messages. The fact that existing tests needed to be changed, shows that the semantics did change.

I do not know if the change is incorrect. But if introduced, according to SemVer it must be introduced in a new major release, to give downstreams the chance to port their code accordingly.

Fixes #99

See also https://github.com/savonrb/savon/issues/935

This reverts commit 6fc97c6048b28aa79d053dc7bf9b736513c42c11.

cc @fernandes @tjarratt

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.04%) to 98.372% when pulling cbcfceda32a2f852a6e6fd7c1639d5347895156f on fsateler:revert-rpc-encoded into 665e78d713ef051ee4bae2dc5e2b1aa135916a17 on savonrb:master.

fernandes commented 3 years ago

hey @fsateler , sorry if it caused any trouble, I was working on a project using savon/wasabi a few years ago and this was (apparently) the fix... as you are using it nowadays and it became a trouble, I'm +1 to merge the fix, I'm still not working on that project anymore, so I can't test this PR

thanks for mentioning me!

arashm commented 3 years ago

Hey, @olleolleolle. What's your thought on this? Can this be merged?

fsateler commented 3 years ago

Hi! Friendly ping. Is there anything I can do to help move this forward?

olleolleolle commented 3 years ago

So, this drops a block of code and the tests still pass. That's suspicious, of course, suspicion about the old code, that is.

The PR description is a little short and does not go into detail about the benefits we get now - that would always help a reviewer.

I agree that we have troublesome parts in current Wasabi and want to work on getting that fixed. Users pin to earlier releases to make their apps work.

I'll reread all the linked issues.

olleolleolle commented 3 years ago

@fsateler Ah, thanks! Now back on track and ready to merge!

olleolleolle commented 3 years ago

@fsateler Thanks for fixing this!

fsateler commented 3 years ago

@olleolleolle thanks to you for maintaining this gem!

arashm commented 3 years ago

Thanks @olleolleolle and @fsateler. Is there any plan to release a new version on rubygems any time soon?

olleolleolle commented 3 years ago

Soon is a strong word. Maybe within a month when I'm back home?

olleolleolle commented 3 years ago

The current release can be used with a git source in Gemfiles, adding robustness to the testedness in the real world, for this upcoming version.

arashm commented 3 years ago

Hey @olleolleolle, sorry for insisting on this, but got the time to release the new version?

brayfe commented 3 years ago

Hello all! Found this thread while investigating SOAP failures for one of my endpoints after upgrading to Savon 2.12.1 and Wasabi 3.6.1. I did as @olleolleolle suggested and used the git source in my Gemfile and that resolved the issues for me. Thank you for the commit!

olleolleolle commented 3 years ago

@brayfe Appreciate the confirmation!