Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18192
Thank you @zhengcanbin and @jerryshao.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
@HyukjinKwon OK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so,
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18192
Okay, I won't go merging it without an explicit +1. @zhengcanbin, would you
mind closing this PR/JIRA, and watching PRs and suggesting this fix latter?
---
If your project is set up for it,
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/18192
I'm neutral to this changes, personally I don't think it is quite necessary
to do such changes here. So my response is +0.
---
If your project is set up for it, you can reply to this email and
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18192
@jerryshao, are you still not fond of this change as is? I was thinking of
suggesting closing this or just merging rather than leaving open.
---
If your project is set up for it, you can reply
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
@zsxwing
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18192
I guess it is discouraged to move codes alone as explained but wouldn't it
be better to merge this rather than close if this looks better in any way and
the change is safe?
---
If your
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
@jerryshao Should I close this issue ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/18192
The change should be safe, but usually we don't do such code structure
refactoring alone without a strong reason, so I'm neutral of this change.
---
If your project is set up for it, you can
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
@jerryshao It's a tiny change for more reasonable code structure. There
exists three `ShuffleWriter ` implementations, we first use the helper method
`SortShuffleWriter#shouldBypassMergeSort`
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/18192
@zhengcanbin can you please clarify the benefit of your changes? I don't
see a big difference here.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
Tks @HyukjinKwon
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/18192
I think he suggested to fix the title to be complete (the current title of
this PR looks truncated with "...". ) and describe what this PR proposes in the
PR description with the PR template -
Github user zhengcanbin commented on the issue:
https://github.com/apache/spark/pull/18192
@heary-cao Tks, and what title you suggest and which comment format is
correct ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well.
Github user heary-cao commented on the issue:
https://github.com/apache/spark/pull/18192
Suggest modifying the title and Comment format is incorrect.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/18192
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this
16 matches
Mail list logo
