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

Introduce build setting for current Scala version #1558

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Conversation

aszady
Copy link
Contributor

@aszady aszady commented Mar 25, 2024

Description

Build setting & config setting to transition and query about the Scala version.

A small change compared to #1552 is that I don't allow an empty string value for the build setting.

This shouldn't change any behavior.

Motivation

Originally #1290.
Partitioned from #1552.

Copy link
Collaborator

@simuons simuons left a comment

Choose a reason for hiding this comment

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

Thanks. Overall looks good. Please check minor comments.

scala_config.bzl Outdated Show resolved Hide resolved
scala_config.bzl Outdated Show resolved Hide resolved
@simuons
Copy link
Collaborator

simuons commented Mar 26, 2024

Also big thanks for

don't allow an empty string value for the build setting.

that was puzzling me in previous pr.

scala_config.bzl Outdated
@@ -39,8 +50,19 @@ def _store_config(repository_ctx):
"ENABLE_COMPILER_DEPENDENCY_TRACKING=" + enable_compiler_dependency_tracking,
])

build_file_content = "\n".join([
'load("@bazel_skylib//rules:common_settings.bzl", "string_flag")',
"string_flag(",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should keep this as string_setting for now and do not allow to change this via command line? wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I see no use case for a flag at the moment.

Co-authored-by: mkuta <mkuta@virtuslab.com>
@aszady
Copy link
Contributor Author

aszady commented Mar 27, 2024

Updated the name of config setting as suggested in #1559 (comment)

@simuons simuons merged commit 2df777e into bazelbuild:master Mar 28, 2024
2 checks passed
@aszady aszady deleted the bs branch March 28, 2024 10:03
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.

None yet

4 participants