-
Notifications
You must be signed in to change notification settings - Fork 616
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
Document deprecation on :attributes! in favour of :@attr => ... in Savon 2 #518
Comments
Thanks for pointing this out, @skalarproduktraum -- would you mind making a Pull Request on the docs if there's an area that covers that? Alternatively, this sounds like it might be an actual bug related to changes in gyoku, although I would be surprised if there wasn't a test for this in Savon... Either way, this needs some investigation, and a failing test case would probably be useful for starters. Would you mind attaching a gist of something that fails to work as you expect? |
Hi @tjarratt, actually, there is no test in the v2 branch that uses attributes. I'm a bit in a hurry currently, but I'll prepare a test or gist later today or tomorrow. If it's no bug, it'd nevertheless be nice to document the changes, as this took me an awful lot of time to figure out :-/ |
I finally managed to write a test for Savon 2, which makes a request (which is expected to fail) and captures the request sent, checking for absence of an Sorry that the test for the new syntax is |
Interesting, I'll have to take a further look into this when I get some time. I really appreciate you taking a look into this, @skalarproduktraum ! |
@skalarproduktraum I'm terribly sorry that it took me so long to take a look at this! The tests you added are great, and exercise a rather critical part of Savon that hadn't really been tested in an integration environment yet, so I'd love if we could merge that into the 2.0 branch of Savon. Would you mind issuing a pull request? |
Hi there @tjarratt! Sorry for taking so long, I kinda missed the notification about your request :( I've finally submitted the pull request for the attribute test. Please let me know if there is something else I can do. |
Cool, glad we could get that merged in :) |
savonrb/savon#518 causes the attribute handling to break. This is a first pass at moving all the attribute parsing to the new behavior.
As per savonrb/savon#518, changes to Savon causes the gem to break in version of Savon beyond 2.2. This is a first pass at moving all the attribute parsing to the new behavior.
As per savonrb/savon#518, changes to Savon causes the gem to break in version of Savon beyond 2.2. This is a first pass at moving all the attribute parsing to the new behavior.
# Conflicts: # lib/marketingcloudsdk/soap.rb # spec/soap_spec.rb From johnadamson/FuelSDK-Ruby per savonrb/savon#518 and salesforce-marketingcloud#7
I just figured out that the
{ :element => ..., :attributes! => { :element => {...}}
syntax does not work anymore in Savon 2, seemingly due to changes in Gyoku. It'd be nice if that would be documented somewhere.Using the
{:element => { :@attrib => ...}}
syntax works like a charm.The text was updated successfully, but these errors were encountered: