Strip some uses of @JvmStatic from runtime #3487
Merged
+0
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
The API compatability tests are failing with I'm surprised that shows up in the api diff, attempting to access it from Java I get what I expected:
I can either add back the @JvmStatic or take it out of the Or, seeing as it is both unused and internal, I can remove the companion/operator entirely and remove it from the API |
Its internal so this should be a safe removal? I'm vaugely sure FieldEncoding#get could just be removed wholesale. Also dumped api to remove, as it was internal and safe to kill.
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
force-pushed
the
jvm_static_stripping
branch
from
315cd68 to
6869218
Compare
Contributor
Author
Updated the commit that killed it from FieldEncoding, thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm vaguely confident none of these
@JvmStaticusages 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:
./gradle build