Skip to content

Conversation

@ueshin
Copy link
Member

@ueshin ueshin commented Nov 15, 2014

No description provided.

@SparkQA
Copy link

SparkQA commented Nov 15, 2014

Test build #23424 has started for PR 3285 at commit 8a0b059.

  • This patch merges cleanly.

@ueshin
Copy link
Member Author

ueshin commented Nov 15, 2014

I'm sorry but I'm not sure if we may use property for artifactId.
What do you think about this, reviewers?

@srowen
Copy link
Member

srowen commented Nov 15, 2014

Yes, I thought this was discussed and this doesn't work?

@SparkQA
Copy link

SparkQA commented Nov 15, 2014

Test build #23424 has finished for PR 3285 at commit 8a0b059.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23424/
Test PASSed.

@markhamstra
Copy link
Contributor

@srowen is correct, this kind of parameterized artifactId has already been considered but is not permissible. Please close this PR.

@ueshin
Copy link
Member Author

ueshin commented Nov 15, 2014

I see. I'll close this but can I have the pointer where you discussed about this?

@markhamstra
Copy link
Contributor

That was quite awhile back and would require some searching of the mail archives and repo from when Spark was in the Apache incubator. I did what you did soon after we made the switch to Scala 2.10 (https://www.mail-archive.com/commits@spark.incubator.apache.org/msg01573.html), and this does work okay for local builds. But when it comes time to make a public release, the release tools will start complaining because artifactId's in released POMs must be fixed/stable. In later discussions, @pwendell and I determined that living with the annoyance of hard-coded Scala versions in the artifactId's was the best approach short of more involved POM re-writing tools.

@srowen
Copy link
Member

srowen commented Nov 15, 2014

@ueshin Have a look at this discussion, which was related at least: #2318 I can't find the thread where I believe Prashant said he found this doesn't work. (Try changing it locally and see what your installed POM looks like?)

@ueshin
Copy link
Member Author

ueshin commented Nov 15, 2014

@markhamstra, @srowen, Thank you for the pointing. Now I'm closing this.

@ueshin ueshin closed this Nov 15, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants