Skip to content

Do not expose skiko library #689

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

Draft
wants to merge 3 commits into
base: jb-main
Choose a base branch
from

Conversation

MatkovIvan
Copy link
Member

@MatkovIvan MatkovIvan commented Jul 18, 2023

Proposed Changes

Skiko library is currently included as api that makes possible using this unstable API without explicit adding the library.
It's even used in the official samples that might lead to widespread usage of it.
Since it's an implementation details, it should not be exposed by default. In the same time we need to allow users use it "as is" by adding skiko as explicit dependency to their project.

API Change

Skiko library is not exported as the library API anymore. If you need to use org.jetbrains.skiko.* or org.jetbrains.skia.*, please include this library directly into the project.
Compose plugin contains DSL to avoid version conflicts

Issues Fixed

Fixes JetBrains/compose-multiplatform#3152

@MatkovIvan MatkovIvan requested a review from igordmn July 18, 2023 15:42
@@ -117,7 +117,7 @@ if (AndroidXComposePlugin.isMultiplatformEnabled(project)) {
skikoMain {
dependsOn(commonMain)
dependencies {
api(libs.skikoCommon)
implementation(libs.skikoCommon)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Native tests are failing

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. We need to revisit our API because there are a few skiko types "leaks" into public

@@ -94,7 +94,7 @@ if(AndroidXComposePlugin.isMultiplatformEnabled(project)) {
skikoMain {
dependsOn(commonMain)
dependencies {
api(libs.skikoCommon)
implementation(libs.skikoCommon)
Copy link
Collaborator

@igordmn igordmn Jul 18, 2023

Choose a reason for hiding this comment

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

Could you also add a section API Change to the message? The plan is to make a link to this PR in the CHANGELOG.

P.S. Probably will be better to do that in the DSL PR in compose-multiplatform repo

Copy link
Member Author

Choose a reason for hiding this comment

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

@MatkovIvan MatkovIvan marked this pull request as draft July 19, 2023 08:02
@MatkovIvan MatkovIvan force-pushed the ivan.matkov/do-not-expose-skiko branch from 8236565 to 22fe418 Compare July 19, 2023 08:24
MatkovIvan pushed a commit that referenced this pull request Sep 10, 2024
set-output is now deprecated. See: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

Test: GH Workflows

This is an imported pull request from androidx#689.

Resolves #689
Github-Pr-Head-Sha: 13e0e20
GitOrigin-RevId: a020a5f
Change-Id: Iba9cfd114e922a52ca737345a072f8309275430c
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.

org.jetbrains.skia.* shouldn't be exposed by default
2 participants