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

Code cleanups #4542

Merged
merged 1 commit into from Jan 30, 2022
Merged

Code cleanups #4542

merged 1 commit into from Jan 30, 2022

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Jan 29, 2022

  • Remove redundant SAM.
  • Proper accessors in kotlin.
  • Remove unneeded generic type.
  • Inline local variable.
  • Const values.
  • Sort in kotlin.
  • Until in kotlin.

cortinico
cortinico approved these changes Jan 30, 2022
View reviewed changes
Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Overall looks good 👍 Just a minor nit

detekt-gradle-plugin/src/functionalTest/resources/jvm/src/main/kotlin/Errors.kt
@@ -8,6 +8,7 @@ class Errors {
}

fun javaExit() {
@Suppress("ReplaceJavaStaticMethodWithKotlinAnalog")
Copy link
Member

Choose a reason for hiding this comment

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

nit: Why this? Was it triggering a warning of any form? If so where?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I see a suggestion by IDEA to replace System.exit(1) with exitProcess(1), it's not proper right here?

Copy link
Member

Choose a reason for hiding this comment

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

it's not proper right here?

I think it's ok. This is a test resource, so we're not actually executing it and should be treated as plain-text. However, as the IDE is recognizing that is Kotlin code, it's probably ok to have it.

@cortinico cortinico added the housekeeping Marker for housekeeping tasks and refactorings label Jan 30, 2022
@cortinico cortinico added this to the 1.20.0 milestone Jan 30, 2022
@cortinico cortinico merged commit 10e9e4c into detekt:main Jan 30, 2022
@Goooler Goooler deleted the clean branch January 30, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cortinico cortinico cortinico approved these changes

@schalkms schalkms schalkms approved these changes

Assignees
No one assigned
Labels
housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Goooler @cortinico @schalkms