PG1204 commented on PR #6075: URL: https://github.com/apache/texera/pull/6075#issuecomment-4941959809
> LGTM. Clean, behavior-preserving unification — the helper is now a plain function of its inputs, and making `validation` required means every caller has to supply it. Also good that this picks up the earlier feedback (dropped the redundant `mockClear`, kept the red-paint test). > > One question, not on the code: the PR says "Closes #5726", but per your own scope note it doesn't implement the "painted exactly once" behavior the issue asks for — the border still gets painted twice on add. Your reasoning for not doing single-paint (the #3614 immediate-render requirement) makes sense, but could you confirm with whoever filed #5726 that descoping it this way is fine before it auto-closes? Otherwise it might be better to keep the issue open for the actual dedup. @Xiao-zhen-Liu thanks for the review. i filed #5726, so confirming directly: descoping to the safe unification is intentional. I changed this PR's description to "Part of #5726" so it won't auto-close, the exactly-once dedup stays tracked in #5726 for if/when it's worth the #3614 risk. This PR just makes the operator-add path obtain its Validation directly and removes the fallback. -- 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]
