-
Notifications
You must be signed in to change notification settings - Fork 615
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
Use proper namespaces for message content #905
Conversation
Can this be merged, please??? |
Didn't notice this when I added my PR #917. That PR also has a breaking test. Would be very helpful for one of the two PRs to be merged. |
Given the slow pace of incorporating changes, I've created a gem that patches this issue. All you have to do is include it in your gem 'savon_fixes' You can find the gem here: https://github.com/lukaso/savon_fixes. It currently covers off these issues and PRs: #916, #899, #820, #895, #862, #905 and #549 (they are all duplicates). I haven't yet tested it in anger, but all tests pass, so please feedback if there are any issues. Adaptations of PRs here are also welcome. |
@pcai @tjarratt @rubiii We do have the same issue, and we have been hacking things along. Should there be any other governance model, given that the gem seems to be on palliative care? This gem has a significant amount of downloads so it's a major security issue to onboard new people or change ownership. But there has to be something the community can do to help?... Also, please consider disabling the stale bot. |
👋 Thanks for all the work on fixing things, it really helps. ❤️ In order to merge this, even if it is a thing that looks like it's fixing outstanding issues, the test suite has not been amended with new cases. So, to help this change get merged, add some kind of test (that would fail without this change) to the suite. |
This is a duplicate of #917 which has been merged (with test). This should be closed. |
closing per prior comment |
Fixes #899 #820 #895 #862