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-518471791
 
 
   Thanks for the detailed voice, @srowen. Invaluable inputs showing how much 
you've been considering deeply on this topic. I totally agree on your points.
   
   I have to say again, my bad I admit I was a bit rushed on this: not all of 
my 2 cents apply to this PR but I made confusion that I'm complaint on these 
things for this PR. I have been struggled with getting review on major PRs for 
fairly long time and it still doesn't work (that's the main concern, honestly) 
so as a workaround I have to pick up minors hoping these minors would pass 
review phase easily, but I failed to isolate the root cause of my concern, and 
expressed concerns on the wrong place. Apologize to @dongjoon-hyun on this 
again. Sorry about that.
   
   Likewise "synchronization of judgement" I still think providing consistent 
view among reviewers would be ideal to contributors. It's natural for reviewers 
to have different perspectives (sure I'm not blaming this. it's natural.), but 
once it's happening, ideally discussion should happen among reviewers (maybe 
including author as well if necessary) and guide as one direction to let 
contributors follow up without confusion. 
   
   It's pretty hard to get rid of individual's preference while reviewing, but 
it's ideal to try out - asking ourselves again what's the current practice of 
this project for this guide. Trying to educate without some documented guide 
might bring unintended reaction - feeling the guide as required to follow one's 
preference - and it wouldn't gonna work, really. That's the reason style guide 
exists, PR template exists, guide doc exists, etc.
   
   Btw, maybe this would help reducing efforts if we do update PR's 
title/description for what we changed during review phase when we are just 
about to merge. Syncing-up PR's title/description with code changes would be 
non-trivial effort, especially code changes is still happening. If 
back-and-forth happens, experience of contributors would be going pretty bad, 
has to fix both code and PR. At least we could try to avoid this.

----------------------------------------------------------------
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]

Reply via email to