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 support for kt_android_local_test #451

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

Bencodes
Copy link
Collaborator

Adding support for Kotlin and android_local_test. Things that changed in this PR:

  • The friends attr has been moved from the kt_jvm_test group to the common group. This is to support creating kt_jvm_library bundles that can be packaged up us a dependency in a test target.
  • compile.bzl was updated to enforce that testonly targets have friends associated with them. This was to keep compatibility with the old behavior.

@cgruber
Copy link
Collaborator

cgruber commented Jan 15, 2021

Glad you've done this - I've been too busy to get to freinds support. :(.

That said, we had a bunch of conversations about this in a few places, and we were thinking of taking this moment to change from friends to associate_to, to match the terminology of the gradle plugin, the main other way people use kotlinc.

I'll need to dig into the review to see how you're handling the actual details. Just wanted to start with an initial comment about naming.

@cgruber
Copy link
Collaborator

cgruber commented Jan 15, 2021

Given that this is not really expanding the functionality or availability of friends, really just keeping kt_android_local_test having parity, I'm inclined to not push to expand this much. I can roll my extended freinds support PR on top of this, once it's merged.

Copy link
Collaborator

@cgruber cgruber left a comment

Choose a reason for hiding this comment

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

Given the fact that kt_jvm_library needs this feature for test-lib handling inside of kt_android_local_test, this is regrettable, but necessary. So let's just move on with it.

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.

3 participants