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
Bug/sap xml parsing #1788
Merged
Merged
Bug/sap xml parsing #1788
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 26da7a6.
We try to avoid using Nokogiri in modules due to the sometimes uncomfortable dependencies it creates with particular compiled libxml versions. Also, the previous parse_xml doesn't seem to be correctly skipping item entries with blank names. I will paste the test XML in the PR proper, but do check against a live target to make sure I'm not screwing it up.
working as expected:
merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The spot test data I was using just to validate parse_xml in isolation is below. However, do please validate that the module still functions as you'd expect. You should be able to just run it as-is.
(CC @nmonkee and @jvazquez-r7)
Incidentally, here are the other modules that rely on Nokogiri:
Those should be fixed, too (but they're not using
Nokogiri::XML
which is at the root of the original problem implied in 26da7a6 )