You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2019/08/05 02:05:18 UTC

[GitHub] [spark] HeartSaVioR edited a comment 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

HeartSaVioR edited a comment 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-518059910
 
 
   I understand the intention and appreciate the continuous effort on improving quality of commit message. Thanks!
   
   There're some points I honestly have 2 cents: 
   
   1) The definition of 'good' varies to everyone. One example: now the commit title is far from guide in `git commit`, 50 characters vs 150+ characters. Even without tagging it exceeds 100 characters. 
   When contributors may not agree the guide of revising commit title or description (when they are having different perspective of 'good') but just follow the guide and change the title/description to let PR just merge in, nothing would be changed in further PR. We can't expect it even we put effort to educate them. If we think something is mandatory to be included in the PR's title or description, we should discuss and document it explicitly. (Like #25310 - actually I'm not sure I could agree requiring contributors even describe "developer-facing" change though. Happy to see it excluded.)
   
   2) The project seems to go on direction for asking more and more efforts on contributors, even on minors. Time is gold, not only for reviewers, but also for contributors. We may need to find some balance here.
   
   3) We might want to always remind that we are not doing like reviewing minors with microscope while skimming the code change for 1000+ of lines. So many developers agreed on this old tweet: https://twitter.com/iamdevloper/status/397664295875805184 (It would be fun to go through the thread.) I'd rather say "importance" should be considered instead of "lines", but minors are minors, both code lines and importance.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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