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

Add scala_version function #15

Merged
merged 1 commit into from
Mar 1, 2016
Merged

Conversation

johnynek
Copy link
Member

@johnynek johnynek commented Mar 1, 2016

this is related to #14

We give a function scala_version and a scala_mvn_artifact that can be used in name mangling for scala jars. This can help deal with making WORKSPACES less tied to a particular version of scala.

@johnynek
Copy link
Member Author

johnynek commented Mar 1, 2016

@dinowernli what do you think of this?

"""return the scala version for use in maven coordinates"""
return "2.11"

def scala_mvn_artifact(artifact):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering, should we really be parsing the artifact and stitching it back together? It seems like it might be more natural to just explicitly pass in the parts, but maybe I'm missing something.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that such stringly typed code is not great. That said, see:

https://github.com/bazelbuild/rules_scala/pull/15/files#diff-09498dbadf45966909850dc8a47ebb13R9

bazel's maven_jar expects the artifact in one big string.

We saw massive pains at Twitter with pants when we had _2.9.2 baked into artifact names rather than just one function we could update. Want to make sure the same does not happen here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, fair enough ;)

@dinowernli
Copy link
Contributor

LGTM

johnynek added a commit that referenced this pull request Mar 1, 2016
@johnynek johnynek merged commit 8745399 into master Mar 1, 2016
@johnynek johnynek deleted the johnynek/add_scala_version_fn branch March 1, 2016 18:41
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.

4 participants