-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New AGP target migration #5466
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
New AGP target migration #5466
Conversation
...lugins/compose/src/test/kotlin/org/jetbrains/compose/test/tests/integration/ResourcesTest.kt
Outdated
Show resolved
Hide resolved
...s/compose/src/test/test-projects/misc/androidAppWithResources/featureModule/build.gradle.kts
Show resolved
Hide resolved
gradle-plugins/compose/src/test/test-projects/misc/commonResources/build.gradle.kts
Show resolved
Hide resolved
|
|
||
| sourceSets { | ||
| commonMain.dependencies { | ||
| api(compose.runtime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plugin aliases are depreceted, please use version catalog instead. Replace version dynamically based on CI params if needed (see Viktor's PR as example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be done as part of Victor's work, I guess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not. It's already deprecated, so adding new setup with deprecated approach doesn't sound good
cc @kropp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it to be consistent with other test projects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm updating existing tests in #5469, so it now makes sense to align new test too
...le-plugins/compose/src/test/test-projects/misc/fullTargetsAppWithResources/gradle.properties
Outdated
Show resolved
Hide resolved
| } | ||
| dependencyResolutionManagement { | ||
| repositories { | ||
| maven("https://maven.pkg.jetbrains.space/public/p/compose/dev") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the order is different from repositories block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't matter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might matter. Especially the order of maven-local and our dev, since we have https://youtrack.jetbrains.com/issue/CMP-8850 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I know it matters in a gradle logic. but in our tests projects there is a lot of such mess
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to fix what I find
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. I won't
CMP-9182 Refactor settings.gradle in tests projects
.../compose/src/test/test-projects/misc/fullTargetsAppWithResources/androidApp/build.gradle.kts
Outdated
Show resolved
Hide resolved
.github/workflows/gradle-plugin.yml
Outdated
| gradle: ['8.7', '9.0.0'] | ||
| agp: ['8.6.0', '8.9.0', '9.0.0-alpha01'] | ||
| gradle: ['8.13', '9.2.0'] | ||
| agp: ['8.11.2', '8.12.3', '9.0.0-alpha11'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done on TeamCity as well. Please
- Make the changes inside
compose-teamcity-configrepo inside the branch with the same name - Open PR on space to review that part too
- Run
SNAPSHOTbuild from your branch to make sure that it's green
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…orm.library` in CHR test projects; migrate from `android` to `androidLibrary` configuration.
…ha11 in test matrix.
…ion for `misc/appleResources`.
474752a to
2b7e3fa
Compare
…skip Android-specific validation.
…oidLibraryTarget`; re-enable specific `ResourcesTest` assertion.
… FIXME for tracking.
Update test AGP and Gradle versions and all test projects.
gradle:
['8.7', '9.0.0'] -> ['8.13', '9.2.0']agp:
['8.6.0', '8.9.0', '9.0.0-alpha01'] -> ['8.11.2', '8.12.3', '9.0.0-alpha13']Fixes CMP-9162 Bump AGP 9.0 latest alpha tests
Testing
Integration tests
Release Notes
N/A