Skip to content

Conversation

@cbrunsdon
Copy link

I'm vaguely confident none of these @JvmStatic usages are required and can all be safely removed.

I'm not exactly swimming in kotlin or KMP experience but getting a handle on the internal.@Jvm* usages from various PR's and issues like this one led me down a few rabbit holes so figured I'd send the PR while I was poking around and getting an understanding of its purpose/usage.

Disclaimers:

  • Got as many gradle tasks/test to run/build as I could before running into various things that wanted me to have an android SDK,, but haven't ran a full ./gradle build
  • I hope this fits under the "small contributions" clause of the "we don't want commits" from the website
  • Signed the contributor agreement

Its internal so this should be a safe removal?

I'm vaugely sure FieldEncoding#get could just be removed wholesale.
This is nonJvm so looks reasonable?
The @JvmStatic on jvmMain's ProtoAdapter still exposes newMapAdapter, so
this shouldn't be required in commonMain as well
@cbrunsdon
Copy link
Author

The API compatability tests are failing with

-	public static final fun get$wire_runtime (I)Lcom/squareup/wire/FieldEncoding;

I'm surprised that shows up in the api diff, attempting to access it from Java I get what I expected:

Cannot access 'fun get(value: Int): FieldEncoding': it is internal in 'com.squareup.wire.FieldEncoding.Companion'.

I can either add back the @JvmStatic or take it out of the .api, whichever you prefer. It wasn't marked with the internal.@JvmStatic which started my rabbit-holing.

Or, seeing as it is both unused and internal, I can remove the companion/operator entirely and remove it from the API

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.

1 participant