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

Detekt #786

Merged
merged 1 commit into from
Jun 14, 2019
Merged

Detekt #786

merged 1 commit into from
Jun 14, 2019

Conversation

mikepenz
Copy link
Owner

  • add detekt to project
  • enable detekt on travis

Alternative to spotless. Replacing #785

* enable detekt on travis
@mikepenz mikepenz mentioned this pull request Jun 14, 2019
@mikepenz mikepenz merged commit 54ac68b into develop Jun 14, 2019
@@ -10,7 +10,7 @@ buildscript {
compileSdk: 29,
buildTools: "29.0.0",
minSdk : 14,
targetSdk : 29
targetSdk : 28
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this reverted to 28?

Copy link
Owner Author

Choose a reason for hiding this comment

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

roboelectric does not support 29 yet :/

}
dependencies {
classpath 'com.android.tools.build:gradle:3.4.1'
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:${versions.kotlin}"
classpath 'com.github.dcendents:android-maven-gradle-plugin:2.1'
classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.4'
classpath "io.gitlab.arturbosch.detekt:detekt-gradle-plugin:1.0.0-RC15"
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to put version within ext.versions since it occurs in 3 places

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes true. I will do that.

@AllanWang
Copy link
Contributor

Does the formatter actually work? The detekt command itself just seems to label some warnings. It might take detektIdeaFormat to fix it though

@mikepenz
Copy link
Owner Author

I tried locally with wrong indents and it failed the build, so it should fail the travis build too when the wrong indents are used for example.

the idea stuff requires IDEA with the format.sh script which is not available on travis

it will not modify anything

@AllanWang
Copy link
Contributor

Weird. For #788 it doesn't fail to build, even if I move some indents around

47 kotlin files were analyzed.

Ruleset: comments
Ruleset: complexity
Ruleset: empty-blocks
Ruleset: exceptions
Ruleset: formatting - 30min debt
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...SelectExtension.kt:163:64
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...SelectExtension.kt:167:62
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...AbstractItem.kt:60:42
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...AbstractItem.kt:69:45
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...AbstractItem.kt:78:46
	NoBlankLineBeforeRbrace - [PsiWhiteSpace] at /...OnBindViewHolderListenerImpl.kt:72:14
Ruleset: naming - 5min debt
	MatchingDeclarationName - [IInterceptor.kt] at /...IInterceptor.kt:1:1
Ruleset: performance
Ruleset: potential-bugs
Ruleset: style

Overall debt: 35min

Complexity Report:
	- 4317 lines of code (loc)
	- 2297 source lines of code (sloc)
	- 1595 logical lines of code (lloc)
	- 1453 comment lines of code (cloc)
	- 488 McCabe complexity (mcc)
	- 7 number of total code smells
	- 63 % comment source ratio
	- 305 mcc per 1000 lloc
	- 4 code smells per 1000 lloc

Project Statistics:
	- number of properties: 193
	- number of functions: 282
	- number of classes: 47
	- number of packages: 7
	- number of kt files: 47

Successfully generated HTML report at /...
Successfully generated Checkstyle XML report at /...

detekt finished in 1635 ms.

@mikepenz
Copy link
Owner Author

@AllanWang I checked. The problem was that we were within the 10 warnings threshold per module which was ok.

I set it now to 0 and fixed all the remaining warnings and adjusted others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants