HeartSaVioR commented on PR #36737:
URL: https://github.com/apache/spark/pull/36737#issuecomment-1150658562

   General comment from what I see in review comments:
   
   I see you repeat the explanation of the code you changed; I don't think 
reviewers asked about the detailed explanation of the code changes. There is no 
"high-level" explanation why it is broken (I roughly see it's from the language 
spec of modulo operation), and also "high-level" explanation how you deal with 
it in this PR. Please look through the description of the reference Flink PR 
you linked - while it also mentioned about code snippet, it explained with high 
level first, and then introduced the code change it proposed.
   
   As long as you update the PR description with high-level explanation, I 
guess it should be straightforward to understand the code change, and you'd 
easily pass the reviews.


-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to