-
-
Notifications
You must be signed in to change notification settings - Fork 759
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
Add Gradle pre-commit hook to docs #6892
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6892 +/- ##
============================================
- Coverage 84.71% 84.70% -0.02%
+ Complexity 3986 3983 -3
============================================
Files 580 578 -2
Lines 12162 12151 -11
Branches 2496 2495 -1
============================================
- Hits 10303 10292 -11
Misses 625 625
Partials 1234 1234 ☔ View full report in Codecov by Sentry. |
0d1b761
to
6ceba8d
Compare
Any ideas what Vercel error is and how can I fix it? I've only updated markdown files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding this @matejdro !
@@ -80,3 +80,112 @@ if [ $EXIT_CODE -ne 0 ]; then | |||
exit $EXIT_CODE | |||
fi | |||
``` | |||
|
|||
## Only run on staged files - Gradle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part should be moved before the CLI, as the initial script suggests to use ./gradlew
.
So this page will look like:
- Run
gradlew
on everything - Run
gradlew
only on modified files - Run CLI only on modified files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to the top
|
||
## Only run on staged files - Gradle | ||
|
||
If the CLI is not your cup of tea (you don't want to keep separate CLI binary set up, you want type resolution etc.), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the style we use in the rest of the docs, could we simplify a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated that part.
If the CLI is not your cup of tea (you don't want to keep separate CLI binary set up, you want type resolution etc.), | ||
you can also configure Gradle detekt to only run on staged files. | ||
|
||
First, we need to declare `GitPreCommitFilesTask` task - a gradle task that will retrieve list of staged files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be useful to specify where to create this file for folks that are not used to precompiled script plugins
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated that part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cortinico Is there something left to do in this PR to get it merged?
Otherwise, we should give it a go, since it improves detekt's doc.
d441b4b
to
b2b50c4
Compare
@schalkms yes versioned docs were missing for all the 1.23.x versions. |
By the way, since making this PR, I have found a couple of improvements to this approach:
If you want, I can include those on Monday and it can be merged after that. |
Yes please 👍 |
64654a2
to
d234fd3
Compare
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comments also apply to the versioned files
```kotlin | ||
tasks.withType<io.gitlab.arturbosch.detekt.Detekt>().configureEach { | ||
if (project.hasProperty("precommit")) { | ||
dependsOn(gitPreCommitFileList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? And, if so, where is this task defined?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, forgot to remove this one. Will delete it. Good catch.
This PR is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days. |
Currently, pre-commit hook page only mentions CLI steps. This requires user to maintain separate detekt binary in the project and it's not compatible with type resolution.
With this PR, I have also added instructions on how to only scan staged files with Gradle.
Setup is kinda verbose, since you have to run through several hoops to get it to work with configuration cache. This is also kts only (it can probably be converted to groovy, but I'm not very well versed in groovy).