Skip to content

Conversation

@07ARB
Copy link
Contributor

@07ARB 07ARB commented Dec 10, 2019

What changes were proposed in this pull request?

Wrong 2nd argument type. Found: 'org.apache.spark.sql.Column', required: 'scala.collection.Seq<org.apache.spark.sql.Column>'

Why are the changes needed?

Fixed java file compilation error problem.
Wrong 2nd argument type. Found: 'org.apache.spark.sql.Column', required: 'scala.collection.Seq<org.apache.spark.sql.Column>'

Does this PR introduce any user-facing change?

No

How was this patch tested?

build examples module locally

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

…lumn', required: 'scala.collection.Seq<org.apache.spark.sql.Column>'
@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

@dongjoon-hyun and @srowen, please review this PR

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show how you tested? IIRC, PR builder compiles examples.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

@HyukjinKwon , you mean local build screenshot you want ?

@HyukjinKwon
Copy link
Member

build examples module locally

I want the commands you ran to verify the compilation failure you faced.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

This issue I got to know when I am fixing this problem.
[SPARK-30193][Build]Removed unused import statement from java file. #26819

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

I observed this issue in my IDE when I open this file, not using any command

@07ARB 07ARB requested a review from HyukjinKwon December 10, 2019 02:13
@HyukjinKwon
Copy link
Member

The build seems already fine. It seems even fine in my IDE. It's likely your env issue.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

This changes not required?

@HyukjinKwon
Copy link
Member

Yes, because the build passes fine and it can't reproduce this issue in my IDE. It looks likely the issue in your env.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

OK, no problem

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

There is unused code in java file and exception related warning there in code I will correct the code as per this jira and will raise PR that will be ok?

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

Will change the jira description also after raising PR?

@HyukjinKwon
Copy link
Member

I would file a separate JIRA. Removing unused codes are minor so I don't think it needs a JIRA. I personally don't like to open a PR only to remove unused codes because it makes backporting harder.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

OK fine no problem, thank you 😊

@srowen
Copy link
Member

srowen commented Dec 10, 2019

Hm, yeah I don't see that either. I also don't see how this change could affect the error you cite? it just changes how the same thing is imported.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

I also need to see why it's showing for me and my friends also have same issue

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

But thank you all for giving quick response and guidance.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

One question I have, if I found build got failed for any PR and I tried to fixed that build issue and then created jira for that, it's ok?

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

But at the same time other person raised follow-up PR for that which I don't know, then how you are controlling this thing?

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

Just for my knowledge I am asking because I tried to resolved unused import issue problem and some other person also raised PR for that

@HeartSaVioR
Copy link
Contributor

HeartSaVioR commented Dec 10, 2019

Just file an issue and comment that "I'm working on this". Regarding unused import issue, I've raised a PR two hours earlier than your PR, so in reality you've missed my PR before start to work. Does this explain your concern?

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

OK got it, I was late, and I missed yours PR

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

Once again thanks everyone.

@srowen
Copy link
Member

srowen commented Dec 10, 2019

First, I'd check whether the Jenkins builds are green. If they are, then it's not a Spark problem, almost surely. You can be a little more conservative about opening PRs and JIRAs there.

I'm not sure what you're asking... if someone fixes an issue before you do, is that a problem? no, just happens. It's rare anyway, unless it's an obvious build breakage.

If someone proposes a fix for a JIRA you reported? also not necessarily a problem; work together on the best solution. You can say "I'm working on this" sure, but we don't particularly encourage anyone to own the solution. We do need people to collaborate in good faith towards one solution.

What JIRA are we talking about?

If you're asking about removing unused imports, I wouldn't bother.
Separately Dongjoon was trying to reenable lint checks for unused imports today, but I presume he'd fix all of those in that same PR.

@07ARB
Copy link
Contributor Author

07ARB commented Dec 10, 2019

@srowen, now everything is clear, I will take care of all this suggestions in future.
Any how good learning for me from yesterday night.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants