Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Update samples with the latest changes. #123

Merged
merged 1 commit into from Aug 14, 2018

Conversation

tasomaniac
Copy link
Contributor

  • Remove redundant *.kt ignores for java tools
  • Add ktlint integration to samples
  • dependency updates

- Remove redundant `*.kt` ignores for java tools
- Add ktlint integration to samples
@@ -19,16 +18,6 @@ test-pattern:
- 'SpreadOperator'
- 'TooManyFunctions'

build:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is wrong and already something we mention in the advanced docs

Copy link
Contributor

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

Nice one, just one comment


repositories {
google()
maven { url "https://plugins.gradle.org/m2/" }
gradlePluginPortal()
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

@@ -41,4 +41,8 @@ staticAnalysis {
output = project.file("build/reports/detekt")
}
}

ktlint {
includeVariants { variant -> variant.name.contains('debug') }
Copy link
Contributor

Choose a reason for hiding this comment

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

How come? I understand this may be ok to speed up sample builds but I fear people will just copy-paste this into their own projects and end up not having coverage for release sourcesets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha 🙂 just copy pasted from above.

I do believe that people copy paste these. But keep in mind that this is the variant. So debug and main source folders are checked.

I would say let's merge this and create a new issue overall to discuss what is the best practice here and update all of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but that means release is not checked :P Anyway if we're doing this above too let's just ship it for now, please create an issue to track that we need a better approach (even just a comment would help I think)

@@ -76,4 +75,8 @@ staticAnalysis {
}
}

ktlint {
android true
includeVariants { variant -> variant.name.contains('debug') }
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here

@@ -41,4 +41,8 @@ staticAnalysis {
output = project.file("build/reports/detekt")
}
}

ktlint {
includeVariants { variant -> variant.name.contains('debug') }
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but that means release is not checked :P Anyway if we're doing this above too let's just ship it for now, please create an issue to track that we need a better approach (even just a comment would help I think)

@rock3r rock3r merged commit e9fb5ce into novoda:develop Aug 14, 2018
@tasomaniac tasomaniac deleted the taso/update-samples branch October 2, 2018 10:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants