Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20169
(I take back my comment @gatorsmile. I feel sure I've seen you quickly
commit a change -- which I think is entirely fine in cases where you think it's
appropriate -- but couldn't find an example
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
To be honest, I really care the quality, especially for such an important
infrastructure software. The above argument is not related to the trust at all.
Sorry for any misunderstanding.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
The difference is that I won't care if you get a review from someone else
and and check that code in. Even if you don't ping me. By the fact that you are
a committer, I trust your judgement that you
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
@vanzin Yes. That is nice to have, but not required in the Apache
community.
I believe both of us want to build the best-quality Spark. That is why I
asked whether we should encourage
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
> I think you also notice that Wenchen and I are the top contributors and
reviewers in Spark SQL
No disagreement there.
> If any change made in Spark SQL, I think it would be nice
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
@vanzin I never said I and Wenchen are acceptable peers. I have to correct
it. I think you also notice that Wenchen and I are the top contributors and
reviewers in Spark SQL. If any change made
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20169
@gatorsmile, I see you merge changes with no review frequently, so I'm
surprised to see you say this. I trust your judgment when you do. You've made
the point repeatedly that you think you should be
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
And where exactly was a peer review missing here?
I didn't check this in without review. You're basically saying that only
you and Wenchen are acceptable "peers". I just disagree strongly
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
@vanzin You might not understand my point. We need a peer review, even if
we are familiar with the code base. Let me give a better example for clarifying
my point.
Even if I am
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20169
I think @vanzin is as qualified to modify this code as anyone. I defer to
your and his judgment about who really needs to weigh in, if anyone, before
merging. We don't, and can't, efficiently
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
> I am not familiar with the code base of SHS
Please leave the strawmen out of the discussion. I *am* familiar with this
part of the code base, you just believe I'm not.
---
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
Yeah. That is a valid argument for the new contributors, but it is not true
for the active contributors and committers like you and me.
Let me give an example. I am not familiar with
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/20169
Yes I'm not following, @gatorsmile. You're saying there are no owners but
wanted to revert a change because you felt it was code that should only change
with your review. You're correct that there
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
> That is why I suggested above to ping me and @cloud-fan when the
community does the change in Spark SQL.
The point I'm making (and I believe Hyukjin too) is that it should be the
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
Thank you for your previous contributions~ In the recent few releases,
there are major refactoring in the related codes. The codes do not belong to
anybody. Our goal is to build the best-quality
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
> My only concern is we should notify people who wrote the code
Like, ahem, me? (Check this code history. I've worked on quite a good chunk
of it over time.)
I agree with what
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20169
To be clear, I think I personally usually cc related guys. Yup, if I were
fixing this codes, I would have cc'ed you @gatorsmile because I agree that it's
good to do. Sure, more eyes are better.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
Let me explain my point here. I am not trying to offend anybody. Spark is
becoming more and more complex. Peer review and test cases are the major
methods we used for quality control. Spark is
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20169
> but I don't think it's quite useful to leave cc to few specific
committers as a requirement
I agree with that, we should not be bound to a few specific committers. But
I think it's
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20169
It might be _optionally_ good to leave cc who are related with the proposal
itself for notification purpose but I don't think it's quite useful to leave cc
to few specific committers as a
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20169
A late LGTM. @vanzin I'm sorry that we sometimes missed your pinging, but I
think it's still good to ping more SQL people for SQL changes, for notification
purpose. A post-hoc review is better
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
> Could you please ping me or @cloud-fan when changing the codes in SQL?
I've had really spotty results pinging people. How about you guys make an
effort to monitor changes in SQL if you
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
@vanzin Could we revert this PR since this is not well reviewed?
---
-
To unsubscribe, e-mail:
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20169
Will review this more carefully later.
@vanzin Could you please ping me or @cloud-fan when changing the codes in
SQL? I believe we are more familiar about the code base in this area
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
Merging to master.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86539/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20169
**[Test build #86539 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86539/testReport)**
for PR 20169 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20169
**[Test build #86539 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86539/testReport)**
for PR 20169 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/148/
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/20169
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20169
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85733/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20169
**[Test build #85733 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85733/testReport)**
for PR 20169 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20169
**[Test build #85733 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85733/testReport)**
for PR 20169 at commit
36 matches
Mail list logo
