leehaut commented on PR #8247: URL: https://github.com/apache/hop/pull/8247#issuecomment-5534574325
Thanks @mattcasters — I agree with the split, and with keeping the copy-factory / live `alreadyChanged` changes. On `XmlSnapshotUndo`: you are right. Comparing the entire pipeline or workflow with a DOM walk on the UI thread is too heavy. I will revert `sameXmlContent` to `decompress(left).equals(decompress(right))`. On treating empty strings like `null` in `XmlMetadataUtil` (not writing the tag): that may well be the cleaner long-term fix, and I am not ruling it out. It does change what we persist, though, so I would like to hear from @hansva before we go that way. On #8173 the first attempt omitted empty tags the same as `null`, and the concern then was that, on transform/pipeline upgrades, a missing field and an explicit blank are not always the same thing. I do not want to reopen that without checking. @hansva, would you be OK with skipping empty string tags at serialization time, or should we keep `null` (omitted) and `""` (`<tag/>`) distinct on disk? In the meantime I can land the copy-factory / `alreadyChanged` part and take the full-document XML compare back out, so the undo path stays a simple string equals. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
