Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SPARK-2482: Resolve sbt warnings during build #1330

Closed
wants to merge 1 commit into from

Conversation

witgo
Copy link
Contributor

@witgo witgo commented Jul 8, 2014

At the same time, import the scala.language.postfixOps and org.scalatest.time.SpanSugar._ cause scala.language.postfixOps doesn't work

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@vanzin
Copy link
Contributor

vanzin commented Jul 8, 2014

LGTM if tests pass.

@srowen
Copy link
Member

srowen commented Jul 8, 2014

Were all of those imports being removed not required after all to avoid warnings? as long as that's the case (i.e. no build warnings) yes this is great IMHO.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16410/

@witgo witgo changed the title Resolve sbt warnings during build SPARK-2482: Resolve sbt warnings during build Jul 15, 2014
@pwendell
Copy link
Contributor

Jenkins, retest this pleae.

@pwendell
Copy link
Contributor

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Jul 15, 2014

QA tests have started for PR 1330. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16690/consoleFull

@pwendell
Copy link
Contributor

Could you paste exactly which warnings you are eliminating? I don't see any warnings in our master jenkins build that seem relevant to these changes.

@witgo
Copy link
Contributor Author

witgo commented Jul 16, 2014

As a result of #772. The master has fixed this problem. But we should remove this line <arg>-language:postfixOps</arg> in pom.xml#L807. Once we remove this line.The problem will be back.
May have a look the discussion in #1069

@SparkQA
Copy link

SparkQA commented Jul 16, 2014

QA tests have started for PR 1330. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16702/consoleFull

@SparkQA
Copy link

SparkQA commented Jul 16, 2014

QA results for PR 1330:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16702/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 4, 2014

QA tests have started for PR 1330. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17855/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 4, 2014

QA results for PR 1330:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/17855/consoleFull

@andrewor14
Copy link
Contributor

test this please

@srowen
Copy link
Member

srowen commented Aug 25, 2014

This is a sort of duplicate of #1069 @witgo this was opened a few times?

From the discussion, I am not clear this should be committed. The preferred practice here seems to be to import scala.language.postfixOps rather than use a compiler flag. This does the opposite.

In fact the compiler flag -language:postfixOps should be removed, which would then make these imports being removed important.

@witgo
Copy link
Contributor Author

witgo commented Aug 26, 2014

@andrewor14 , @srowen
This is mainly to solve the problem of importing the scala.language.postfixOps and org.scalatest.time.SpanSugar._ at the same time.

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have started for PR 1330 at commit 15d5ed3.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 3, 2014

QA tests have finished for PR 1330 at commit 15d5ed3.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • protected trait YarnAllocateResponse

@jkbradley
Copy link
Member

@witgo Hi, I was asked to take a look at this PR. I tested the current master vs. this PR merged with the master, and I actually found that this PR added postfix warnings for me. (No postfix warnings in the master, but several with this PR.) Could you please show the warnings you get, plus info on what might be causing those warnings on your system? Thanks!

@witgo
Copy link
Contributor Author

witgo commented Sep 11, 2014

The code has been updated.

@@ -839,7 +839,6 @@
<arg>-unchecked</arg>
<arg>-deprecation</arg>
<arg>-feature</arg>
<arg>-language:postfixOps</arg>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jkbradley I removed this parameter. The related discussion in #1069

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have started for PR 1330 at commit 179ba61.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 11, 2014

QA tests have finished for PR 1330 at commit 179ba61.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Sep 11, 2014

The net-net change for all of these overlapping PRs should be: remove the compiler flag, and, add the import everywhere that the warning occurs. If there are warnings after this PR, they need to be squashed with more imports.

@witgo
Copy link
Contributor Author

witgo commented Sep 11, 2014

No postfix warnings in 179ba61 .

@jkbradley
Copy link
Member

@witgo Sorry, I had not realized that this had not been updated since the discussions. Just tested it, and it worked for me. LGTM

@andrewor14
Copy link
Contributor

Ok, I am merging this over the alternative PR #1323, which is now closed. Thanks to everyone who looked at this.

@asfgit asfgit closed this in 33c7a73 Sep 12, 2014
@witgo witgo deleted the sbt_warnings3 branch September 12, 2014 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants