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

Make JvmStatic opt-in and fix an erroneous usage #417

Merged
merged 2 commits into from
Dec 19, 2020

Conversation

benjamin-bader
Copy link
Collaborator

All this time I've mistakenly believed that @JvmStatic converts a companion-object method into a bona-fide static method. That turns out to be untrue. What it does is add a second static method to the parent class that loads the companion object and calls the annotated method.

R8 optimizes this away, but not everybody uses R8. Since we're moving more and more to a Kotlin-only future, and in the service of (slightly) smaller code, this PR makes @JvmStatic opt-in only via a compiler flag.

It also fixes an erroneous usage of @JvmStatic, where @JvmField is the correct and intended annotation.

@codecov-io
Copy link

codecov-io commented Dec 19, 2020

Codecov Report

Merging #417 (18f6db5) into master (84bd3f5) will decrease coverage by 0.04%.
The diff coverage is 11.11%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #417      +/-   ##
============================================
- Coverage     59.71%   59.66%   -0.05%     
- Complexity      789      793       +4     
============================================
  Files            60       60              
  Lines          5488     5506      +18     
  Branches        860      866       +6     
============================================
+ Hits           3277     3285       +8     
- Misses         1967     1976       +9     
- Partials        244      245       +1     
Impacted Files Coverage Δ Complexity Δ
.../com/microsoft/thrifty/compiler/ThriftyCompiler.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../com/microsoft/thrifty/kgen/KotlinCodeGenerator.kt 82.41% <25.00%> (-0.21%) 145.00 <0.00> (ø)
...main/kotlin/com/microsoft/thrifty/schema/Loader.kt 73.21% <0.00%> (+0.93%) 32.00% <0.00%> (+4.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84bd3f5...18f6db5. Read the comment docs.

@benjamin-bader benjamin-bader merged commit 314fb84 into microsoft:master Dec 19, 2020
@benjamin-bader benjamin-bader deleted the optional-jvmstatic branch December 19, 2020 23:02
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.

2 participants