-
Notifications
You must be signed in to change notification settings - Fork 208
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 x_explicit_api_mode to remaining kotlinc_options #607
Conversation
Support for this was already added here #576. Does it not work? |
No, it's not working, for some reason, although I hadn't even noticed this PR, nor the fact that the options are in fact documented here https://bazelbuild.github.io/rules_kotlin/kotlin.html#kt_kotlinc_options... I'm new to Bazel, but it looks like the |
Hm, the reason I made this change was that my BUILD file does the following load (as per the README):
And AFAICT, core.bzl loads from kotlin/internal, or https://github.com/bazelbuild/rules_kotlin/blob/master/kotlin/internal/opts.bzl, which in turn points to the files I changed in this PR. Assuming I read the code right, I am a complete newbie as mentioned. There seems to be 8 opts.bzl files in the repo (including one named opts.release.bzl). Is that what's causing the issue? |
Ah so the solution here is to replicate the existing |
435339d
to
3efb56f
Compare
3efb56f
to
baa988d
Compare
Done. AFAICT, it was only the 1.5 file that was missing the option. |
Lgtm but @cgruber or @restingbull will need to merge this |
See https://github.com/Kotlin/KEEP/blob/master/proposals/explicit-api-mode.md for more about the switch itself.
I'm not sure how to test this - the best test would be to have some test code with default access modifiers and explicit-api=strict set, and then run a compilation and verify that it fails. That seems complex, and like it would require a fair amount of tooling, which I'm not sure if it's available. Please advice if so.