-
Notifications
You must be signed in to change notification settings - Fork 281
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
Provide select_for_scala_version
utility macro
#1563
Conversation
4dd3950
to
7cfb44f
Compare
Hi, @aszady, would it be possible to write some tests for this macro? I'm not sure about exposing this macro for end users right now. I mean such sophisticated matchers might be needed internally to support wide range of scala versions. But end users (presumably) would have only handful set of scala versions and could write regular selects. Could you make this bzl private for now? I think maybe what's missing is more coarse grained config_settings for scala version at
That would allow to have more stable selects. wdyt? |
@simuons, do you have any specific idea in mind for such tests? I think we cannot test the macro itself, as we haven't enabled multiple versions yet, so such test would be quite trivial.
Certainly it may seem that some matchers are here just for internal support, with most striking example of With the macro we say: this will/won't work for such Scala versions and we usually can state which Scala version caused the change. This information is irrelevant to currently enabled The coarser-grained Scala version settings is something in between, as it emulates the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one way of testing this could be to extract a function that builds a map for select and then test that with https://bazel.build/rules/testing#testing-starlark-utilities
Probably writing analysis test is not feasible right now and those tend to be very tedious.
To be clear: this macro looks nice and is well documented. My only concern was that it might be to powerful for end users and if we could stick just to plain select that would be more familiar.
I have no objections for merging this. Any way it already takes longer than it should and blocks other PRs.
Approving. Thanks.
Description
For later use in BUILD files of
rules_scala
where customization depending on Scala version is needed.End users are also encouraged to use this one.
The approach in this PR differs from the one in #1552, as with selects we will avoid the need of multiplying targets within
rules_scala
for each Scala version configured.This comes at a cost of not so easy access to the internal binaries for a specific, non-default version. #1552 proposed suffixing target names with the Scala version, whereas here the target built depends on a configuration – harder to specify via command-line. Possible workaround for this, if ever needed, would be to use a flag (see: #1558 (comment)).
The choice of matchers is quite arbitrary, but covers the usage required at the moment.
See usage in #1564.
Motivation
Originally #1290.