-
Notifications
You must be signed in to change notification settings - Fork 168
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
Enable using the latest master snapshot version of ktlint #281
Conversation
plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/PluginUtil.kt
Outdated
Show resolved
Hide resolved
plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/PluginUtil.kt
Outdated
Show resolved
Hide resolved
plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/PluginUtil.kt
Outdated
Show resolved
Hide resolved
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.
Just a few small things. Other than that, this looks good. @Tapchicoma anything from you?
For some reason, commit 24d2032 caused the build to fail. I'm fairly new to Kotlin, so I can't see what in the diff could cause that failure. I couldn't reproduce the issue on my machine. Could you please help me understand what's wrong with that commit? |
I just restarted the build, we'll see what's up. |
I would say, that this is more ktlint problem itself. As far as I know it was fixed in Gradle build, but still the issue in the maven builds. |
* master: Add support to disable particular rules via plugin extension. Set default ktlint version to 0.34.2.
It looks like the build is happy now. (!) I merged from master to resolve the conflict. How does it look now? |
@Tapchicoma how do you want to resolve this? |
plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/PluginUtil.kt
Outdated
Show resolved
Hide resolved
@hosamaly I would propose to not merge it for now, if it is not urgent. @shashachu will recheck again if Gradle could be used to publish artifacts on this weekend. |
FWIW we were also hit by this. Is there anything I can do to help get this merged? |
@hosamaly @jacquerie snapshot with proper versioning was published: https://oss.sonatype.org/content/repositories/snapshots/com/pinterest/ktlint/0.35.0-SNAPSHOT/ |
Cool, that works for us! Thanks a lot, @Tapchicoma ! |
@hosamaly sorry for not merging this PR, but proper approach would be to fix snapshots versioning in ktlint repository itself, that is done via pinterest/ktlint#588. Thank you anyway for this PR and raising this problem! |
No worries. Thank you for fixing the root cause. |
I was trying to follow the instructions to use the latest
master
snapshot of ktlint today, but I saw the error about versions less than 0.22 not being supported.This PR enables the use of master snapshots and treats it as the latest version, even though it's original version is 0.0.