oshai / kotlin-logging

Lightweight Multiplatform logging framework for Kotlin. A convenient and performant logging facade.
Other
2.51k stars 110 forks source link

Update Detekt to 1.23.4 #384

Closed AzimMuradov closed 4 months ago

AzimMuradov commented 5 months ago

Should fix issues in #374 automatic PR

AzimMuradov commented 5 months ago

Changes:

I hope this change would make the build script more readable and generally nice to config.

This style guide is already present in the project, so I decided to explicitly describe it in the .editorconfig and detekt.yml. The first one would help an IDE to format the code properly, and the second one will ensure that this style guide would be followed.

AzimMuradov commented 5 months ago

BTW, why does the build script use spotless and detekt at the same time? I think the detekt would be more than enough.

AzimMuradov commented 5 months ago

Also, I could filter out detekt tasks on ...Test source sets if it's needed.

It's as simple as changing

tasks {

    // ...

    afterEvaluate {
        check {
            dependsOn(withType<Detekt>())
        }
    }
}

to

tasks {

    // ...

    afterEvaluate {
        check {
            dependsOn(withType<Detekt>().filterNot { it.name.contains("test", ignoreCase = true) })

            // or even:
            // dependsOn(withType<Detekt>().filter { "Main" in it.name })
        }
    }
}
oshai commented 5 months ago

BTW, why does the build script use spotless and detekt at the same time? I think the detekt would be more than enough.

If I remember correctly we started with detekt and now use spotless (don't remember why I changed it...). So I am not sure we need detekt at all.

AzimMuradov commented 5 months ago

BTW, why does the build script use spotless and detekt at the same time? I think the detekt would be more than enough.

If I remember correctly we started with detekt and now use spotless (don't remember why I changed it...). So I am not sure we need detekt at all.

As far as I know, detekt is a full blown static analysis tool while spotless is more like a formatter/code style checker.

If you want, I can remove detekt from the config. If so, should I just rename this pr, or should I close it, and open the new one?

AzimMuradov commented 5 months ago

The reason I created this PR is because old detekt blocks the addition of the wasmJs target. If you don't currently want to remove detekt we can think about that later and for now I'll just update it's version.

oshai commented 4 months ago

Let's stay only with spotless and remove detekt.