mattcasters commented on PR #7498: URL: https://github.com/apache/hop/pull/7498#issuecomment-4946783183
Hi Lance — thanks for catching this. The inverted condition in the ValueMapperMeta copy constructor is a real bug: non-empty targetType values would fall through to the default "String" path, so clone/copy would lose the configured type. The parameterized unit test you added on clone() is a solid way to lock that down for null / empty / non-empty cases. Could you open an issue that briefly covers: • What went wrong — copy constructor used meta.targetType.isEmpty() instead of !meta.targetType.isEmpty(), so a non-empty target type (e.g. Integer) was not copied and defaulted to String • How to reproduce / impact — e.g. configure Value Mapper with a non-String target type, clone/copy the transform (or otherwise go through clone()), observe target type reset to String • Fix summary — invert the condition; unit test coverage for clone Then link it from the PR description (e.g. Fixes #NNNN or Related to #NNNN). That also makes the change easier to find later when someone is digging through release notes or git history. ─── Again, nice catch on a subtle condition flip — once the issue is linked this should be in good shape to merge. Happy to answer questions if anything about the issue is unclear. -- 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]
