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.

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

Configure detekt #227

Merged
merged 8 commits into from Oct 16, 2019
Merged

Configure detekt #227

merged 8 commits into from Oct 16, 2019

Conversation

CherryPerry
Copy link
Collaborator

Fixes #204

Still long path to fix / ignore every issue.

@CherryPerry
Copy link
Collaborator Author

Suppressed some issues.

@CherryPerry
Copy link
Collaborator Author

Merge after #233

# Conflicts:
#	reaktive/src/iosCommonMain/kotlin/com/badoo/reaktive/scheduler/MainScheduler.kt
#	reaktive/src/nativeCommonMain/kotlin/com/badoo/reaktive/scheduler/CreateTrampolineScheduler.kt
#	utils/src/jsMain/kotlin/com/badoo/reaktive/utils/clock/DefaultClock.kt
@CherryPerry
Copy link
Collaborator Author

Ready to merge.

@@ -127,6 +129,7 @@ fun <T1, T2, T3, T4, R> combineLatest(
mapper(values[0] as T1, values[1] as T2, values[2] as T3, values[3] as T4)
}

@Suppress("LongParameterList")
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's disable this inspection please

@@ -9,6 +11,7 @@ import com.badoo.reaktive.utils.atomic.update
import com.badoo.reaktive.utils.replace
import com.badoo.reaktive.utils.serializer.serializer

@Suppress("ComplexMethod")
Copy link
Contributor

Choose a reason for hiding this comment

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

And perhaps this should be disabled as well?

@@ -9,6 +9,7 @@ internal abstract class SerializerImpl<in T>(queue: Queue<T>) : Serializer<T> {
private val monitor = Any()
private var isDraining = false

@Suppress("ReturnCount")
Copy link
Contributor

Choose a reason for hiding this comment

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

What about this one? Do we really want it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

return X count in one method.

Copy link
Contributor

@arkivanov arkivanov left a comment

Choose a reason for hiding this comment

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

Hm?

@CherryPerry
Copy link
Collaborator Author

CherryPerry commented Oct 14, 2019

I wanted to discuss it in person, but forgot.
Main point: all these checks are based on some experience, and I think it is good at least to try to follow them. In places, where we can't for some reason follow them - suppress.

LongParameterList for example is suppressed only in operators public interface.
But ComplexMethod and ReturnCount could be fixed in future.

@arkivanov arkivanov merged commit ed2ca70 into badoo:master Oct 16, 2019
@CherryPerry CherryPerry deleted the detekt branch October 18, 2019 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Detekt
2 participants