-
Notifications
You must be signed in to change notification settings - Fork 277
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
scala_library shouldn't have a jvm_flags attribute #669
Comments
I guess it is fine to remove it. It should be easy to migrate since it is a no-op. If people follow the pattern of making their own macro wrapper for rules so they can control default options it would be even easier to migrate: their wrapper can accept the option but just discard it. |
I think bazel people would call this abuse of macros but OTOH I'm not sure
they thoroughly considered the use-case of OSS eco system.
I'll send a PR for this
…On Sat, Jan 19, 2019 at 5:44 PM P. Oscar Boykin ***@***.***> wrote:
I guess it is fine to remove it. It should be easy to migrate since it is
a no-op. If people follow the pattern of making their own macro wrapper for
rules so they can control default options it would be even easier to
migrate: their wrapper can accept the option but just discard it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#669 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFwc01uo-O3VWCT3Ggy1GvXgRwxgSks5vEz1agaJpZM4aJAc8>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
AFAICT scala_library doesn't use
jvm_flags
attribute anymore since we havescalac_jvm_flags
andjavac_jvm_flags
.It's only used by binary/repl/test/junit_test which are binary rules.
I'd like to remove it.
On the one hand it's a breaking change OTOH the current attribute is deprecated for many months now.
@johnynek any objections?
cc @aherrmann since I think he was interested in this attribute from #621
The text was updated successfully, but these errors were encountered: