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

Bump io.gitlab.arturbosch.detekt from 1.23.0 to 1.23.1 #812

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Bump io.gitlab.arturbosch.detekt from 1.23.0 to 1.23.1 #812

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bfc587
Bump io.gitlab.arturbosch.detekt from 1.23.0 to 1.23.1
dependabot[bot] Jul 31, 2023
55d5830
Suppress Detekt warning
Marcono1234 Aug 2, 2023
46e34ce
Merge branch 'master' into dependabot/gradle/io.gitlab.arturbosch.det…
urielsalis Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ plugins {
kotlin("jvm") version "1.9.0"
id("org.jlleitschuh.gradle.ktlint") version "11.5.0"
application
id("io.gitlab.arturbosch.detekt") version "1.23.0"
id("io.gitlab.arturbosch.detekt") version "1.23.1"
}

group = "io.github.mojira"
Expand Down
1 change: 1 addition & 0 deletions src/main/kotlin/io/github/mojira/arisa/ModuleExecutor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class ModuleExecutor(
val moduleName = response.first
response.second.fold(
{
@Suppress("detekt:OptionalWhenBraces")
when (it) {
is OperationNotNeededModuleResponse -> {
if (config[Arisa.Debug.logOperationNotNeeded]) {
Comment on lines +28 to 31
Copy link
Contributor

@Marcono1234 Marcono1234 Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Have suppressed this, because it was apparently reporting that for

is OperationNotNeededModuleResponse -> {
    if (config[Arisa.Debug.logOperationNotNeeded]) {
        ...
    }
}

the braces are optional. However, I assume that means we would have to move the if into the same line as the is ... -> which might decrease readability too much:

is OperationNotNeededModuleResponse -> if (config[Arisa.Debug.logOperationNotNeeded]) {
    ...
}

Or we write it like the following without braces, but that might look a bit weird?

is OperationNotNeededModuleResponse ->
    if (config[Arisa.Debug.logOperationNotNeeded]) {
        ...
    }

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

the second one looks fine to me. if detekt accepts that, i think that'd be better than suppressing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, that OptionalWhenBraces rule is actually deprecated (see documentation), and not enabled by default (we just enable all rules in the build script).

Maybe it is also a bug that this rule now reports this issue, because at least the release notes don't seem to mention anything about this. And on the other hand while the second variant (without braces) seems to be accepted, I am bit surprised about that because I would expect that BracesOnWhenStatements with its default multiLine=consistent should prevent that.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's worth thinking about making the formatting rules less strict then, by not requiring literally every rule to pass? I'd assume there are some presets that are less pedantic.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is https://github.com/detekt/detekt/blob/main/detekt-core/src/main/resources/default-detekt-config.yml (but that also disables some non-style rules); not sure if there are any other default configs.

Maybe we can also just wait and see if the behavior changes in the version released after 1.23.1, whenever that might be.

Expand Down