Pull out SDK into its own module #1462
Conversation
| @@ -1,3 +1,5 @@ | |||
| @file:Suppress("invisible_reference", "invisible_member") | |||
There was a problem hiding this comment.
allows "friend references", i.e., extending internal access across cross project-modules. By default, kotlin only allows code within the same module to access internal members.
There was a problem hiding this comment.
Not K2 friendly: https://youtrack.jetbrains.com/issue/KT-67920
Maybe it is possible to befriend another module's sourceSet with this?
https://github.com/TWiStErRob/repros/blob/main/kotlin/test-fixtures-and-suites-friends/build.gradle.kts
| @@ -42,7 +42,6 @@ dependencies { | |||
| api libs.tools.ninepatch | |||
| api libs.tools.sdkCommon | |||
| api libs.kxml2 | |||
| api libs.junit | |||
| "paparazzi.layoutlib.resources.root", | ||
| configurations.layoutlibResources.singleFile.absolutePath | ||
| ) | ||
| } |
There was a problem hiding this comment.
a bit unfortunate that both the sdk and junit rule needs this; but it makes sense, the actual config in practice would come from the (gradle) plugin. might make sense to evolve the sdk into a builder pattern; i'll mess around with this in a follow-up.
| @@ -329,7 +329,7 @@ public class PaparazziPlugin : Plugin<Project> { | |||
|
|
|||
| private fun Project.addTestDependency() { | |||
| val dependency = if (isInternal()) { | |||
| dependencies.project(mapOf("path" to ":paparazzi")) | |||
| dependencies.project(mapOf("path" to ":paparazzi-junit4")) | |||
There was a problem hiding this comment.
deliberate for now. can query gradle extension config for junit4 vs junit5, but not sure how to keep it generic for other 3rd party wrappers.
| dependencies { | ||
| testImplementationAarAsJar libs.androidx.compose.ui.android | ||
| compileOnlyAarAsJar libs.androidx.compose.ui.android | ||
| compileOnlyAarAsJar libs.androidx.compose.runtime |
There was a problem hiding this comment.
calling out this switcheroo; replacing compileOnlyAarAsJar libs.androidx.activity with this: https://github.com/cashapp/paparazzi/pull/1462/files#diff-697f70cdd88ba88fe77eebda60c7e143f6ad1286bca75017421e93ad84fb87df
There was a problem hiding this comment.
nice, much better dependency
jrodbx
force-pushed
the
jrod/2024-05-17/sdk-module
branch
from
59ea47e
to
98db626
Compare
| @@ -1,3 +1,5 @@ | |||
| @file:Suppress("invisible_reference", "invisible_member") | |||
There was a problem hiding this comment.
Consider using all uppercase to highlight that you're suppressing compiler errors
| @file:Suppress("invisible_reference", "invisible_member") | |
| @file:Suppress("INVISIBLE_REFERENCE", "INVISIBLE_MEMBER") |
There was a problem hiding this comment.
Did you intentionally keep the ...:paparazzi Maven coordinate here?
Consider publishing the -junit4 from the -junit4 module and reintroduce a :paparazzi module that apis both the sdk and the junit4.
| } | ||
|
|
||
| tasks.named("dokkaGfm").configure { | ||
| outputDirectory = rootProject.file("docs/1.x") |
There was a problem hiding this comment.
Amazing that you did all this while keeping 1.x 👏🏻
| @@ -28,7 +28,7 @@ import java.util.logging.Logger.getLogger | |||
| * This logger delegates to java.util.Logging. | |||
| */ | |||
| internal class PaparazziLogger : ILayoutLog, ILogger { | |||
There was a problem hiding this comment.
rename class to match module/logger?
Note that this does not rename the artifacts as that would be a breaking change; this should remain binary compatible until 2.0
Also, many of the files in this changeset are simply file renames/moves, so hopefully not as intimidating to review :)