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.

Sign up for GitHub

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

Jump to bottom

fix detection of android kotlin source directories in AGP >= 7 #733

Merged
merged 1 commit into from
Jan 9, 2024
Merged

fix detection of android kotlin source directories in AGP >= 7 #733

merged 1 commit into from
Jan 9, 2024

Conversation

wakingrufus
Copy link
Collaborator

@wakingrufus wakingrufus commented Dec 20, 2023 •
edited
Loading

increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

@wakingrufus wakingrufus changed the title fix detection of kotlin source directories in AGP >= 7 fix detection of android kotlin source directories in AGP >= 7 Dec 20, 2023
@wakingrufus wakingrufus force-pushed the fix-android branch 6 times, most recently from 2669505 to 636192a Compare December 20, 2023 18:23
@wakingrufus wakingrufus marked this pull request as ready for review December 20, 2023 18:37
@wakingrufus wakingrufus mentioned this pull request Dec 20, 2023
Closed
StefMa
StefMa approved these changes Dec 21, 2023
View reviewed changes
Copy link

@StefMa StefMa left a comment

Choose a reason for hiding this comment

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

Nice one 👏
I looked only at the AndroidPluginsApplier.kt class which is fine IMO.
I also tested it locally on my current project and it works fine 🎉
Gradle 8.5
Kotlin version 1.9.21
AGP 8.2.0
Do you need more info regarding that?

:shipit:

@wakingrufus
Copy link
Collaborator Author

Nice one 👏 I looked only at the AndroidPluginsApplier.kt class which is fine IMO. I also tested it locally on my current project and it works fine 🎉 Gradle 8.5 Kotlin version 1.9.21 AGP 8.2.0 Do you need more info regarding that?

:shipit:

Thanks for verifying! I'll give @JLLeitschuh some time to review it before merging

JLLeitschuh
JLLeitschuh reviewed Dec 25, 2023
View reviewed changes
Copy link
Owner

@JLLeitschuh JLLeitschuh left a comment

Choose a reason for hiding this comment

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

Few questions

plugin/build.gradle.kts Outdated Show resolved Hide resolved
plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/android/AndroidPluginsApplier.kt Outdated Show resolved Hide resolved
plugin/src/test/kotlin/org/assertj/core/api/Assertions.kt Outdated Show resolved Hide resolved
@wakingrufus wakingrufus force-pushed the fix-android branch 7 times, most recently from 3f2abf0 to 03d364b Compare January 4, 2024 14:52
JLLeitschuh
JLLeitschuh reviewed Jan 6, 2024
View reviewed changes
Copy link
Owner

@JLLeitschuh JLLeitschuh left a comment

Choose a reason for hiding this comment

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

LGTM, but there are a few minor suggestions, and a question

plugin/src/test/kotlin/org/jlleitschuh/gradle/ktlint/UnsupportedGradleTest.kt Outdated Show resolved Hide resolved
plugin/src/test/kotlin/org/jlleitschuh/gradle/ktlint/android/KtlintPluginAndroidTest.kt Outdated Show resolved Hide resolved
@JLLeitschuh
Copy link
Owner

Any idea why checking bashisims is failing?

@wakingrufus
Copy link
Collaborator Author

wakingrufus commented Jan 8, 2024 •
edited
Loading

Any idea why checking bashisims is failing?

package resolution error in ubuntu. something similar happened before and I had to manually specify a version to fix it. Ill investigate further

@wakingrufus wakingrufus force-pushed the fix-android branch 3 times, most recently from 03d6148 to e45584b Compare January 8, 2024 20:33
@wakingrufus
fix detection of android kotlin source directories in AGP >= 7
ca166cc 
increase maximum gradle version to 8.5
increase maximum ktlint version to 1.1.0
increase maximum kotlin version to 1.9.21
fix lint failures

fixes #702
based on work in #703

fix bashisms by manually specifying correct version of libdistro-info-perl
@JLLeitschuh JLLeitschuh merged commit 977af89 into main Jan 9, 2024
16 of 17 checks passed
@JLLeitschuh
Copy link
Owner

Nice work! Merged!

This was referenced Jan 20, 2024
Open
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StefMa StefMa StefMa approved these changes

@Tapchicoma Tapchicoma Awaiting requested review from Tapchicoma

@JLLeitschuh JLLeitschuh Awaiting requested review from JLLeitschuh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Android: ktlint[SourceSetName]SourceSetCheck task doesn't take kotlin srcDir into account
3 participants
@wakingrufus @JLLeitschuh @StefMa