srowen commented on issue #25335: [SPARK-28601][CORE][SQL][test-hadoop3.2] Use StandardCharsets.UTF_8 instead of "UTF-8" string representation, and get rid of UnsupportedEncodingException URL: https://github.com/apache/spark/pull/25335#issuecomment-518268166 @dongjoon-hyun @HeartSaVioR My $0.02: I do think it's ideal to keep the PR description up to date as it becomes part of the commit log. I don't know if it was a big deal in this case as the additional change was a minor internal refactoring, and we wouldn't exhaustively list every single change in the description. I don't see harm in the reminder or implementing the one-line edit, so it was fine. It was minor but not nit-picky. This is a judgment call and this is part of a process of synchronizing that judgment. There's a very good point here that minor changes inevitably get more scrutiny because there's less to look at. That's not great but it's nature. If you view the occasional microscopic review as more an exercise in communicating about standards, and something that happens when the substance of the change is straightforward, it makes more sense. I'd also say we might review in more detail for people who are making many good contributions, as that person is growing into a reviewer too and perhaps on the path to committer, whereas it's not worth communicating this for a one-off contributor, and we might just let it go or fix it ourselves. Not great rationalizations, but it's not so bad. You are right that reviewers don't even always agree, which is also just life. It can be frustrating if it's happening on your PR but is part of the process of synchronization. Reviewers do really need to bias towards finding consensus quickly. - We might be able to emphasize these details in the PR template - Reviewers should probably keep in mind the contributor's time, sure, and judge how much another round of minor changes 'costs' vs value - Contributors may just have to put up with a bit of this process of synchronization sometimes and, weirdly, I hope feel more included as a result
---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
