aws-sdk-perl icon indicating copy to clipboard operation
aws-sdk-perl copied to clipboard

Use XML::Hash::XS

Open slobo opened this issue 5 years ago • 9 comments

This makes t/10_responses.t pass

slobo avatar Dec 26 '19 19:12 slobo

So, in this first iteration, we learn that SuppressEmpty => undef behaviour is the only difference that test suite can uncover for 10_responses.t. But, while doing this in perl code is probably fast enough for smaller results at least, it feels rather dirty, so I'll try to petition XML::Hash::XS to add equivalent behaviour.

slobo avatar Dec 26 '19 19:12 slobo

@pplu, with this change, test suite passes on my machine (minus some failure on 01_load.t, but same happens on the base branch, and I hope is unrelated). Let me know if there is anything else we need to do. Thanks!

slobo avatar Dec 28 '19 22:12 slobo

Looks like we should look at merging this for 0.44 (before 0.45), see #372

castaway avatar Feb 26 '20 14:02 castaway

Hmm might clean up the issue I had with a few of the S3 actions where the parsing the XML beheaded it and I had to add in a little extra logic to handle the missing root key.

Would have to look at my notes to see which one.

byterock avatar Feb 26 '20 14:02 byterock

There are a few responses that don't contain the main XML key (there's code in RestXMLResponse to handle it) - would be handy to get rid of XML::Simple regardless

castaway avatar Feb 26 '20 15:02 castaway

@pplu , this PR just merges into your xml2hash branch, so probably safe to accept and continue work in there?

slobo avatar Feb 28 '20 17:02 slobo

Or, should we maybe change the target to be the next release branch to see how it behaves?

slobo avatar Feb 28 '20 17:02 slobo

working into xml2hash seems sane to me, I hadnt seen it was against that one

castaway avatar Mar 02 '20 11:03 castaway

Would love to see this stop-gap make it into a release.

mschout avatar Mar 06 '20 16:03 mschout