-
Notifications
You must be signed in to change notification settings - Fork 1
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 ktlint with custom rules #9
Conversation
* @param lintPaths list of paths relative to the project root to lint (or not lint). | ||
*/ | ||
fun Project.configureLinting(lintPaths: List<String>) { | ||
verifyRootProject { "AWS SDK lint configuration is expected to be configured on the root project" } |
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.
Nit: "AWS SDK for Kotlin" or "Kotlin SDK"
// TODO - is there anyway to align this with the version from libs.versions.toml in this project/repo | ||
val ktlintVersion = "0.48.1" |
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.
Question: Would it work to just use `val ktlintVersion: String by project" and pull it in from the root project? That would mean potentially separate versions across the repos but that's what we have now and we're not (yet) planning to commonize the version numbers right?
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.
We are planning to commonize the version numbers (for ktlint anyway) because we've moved the rules into this plugin/project. Downstream consumers won't need to set ktlint version at all, just the paths to lint. Theres no real reason to make the downstream consumers set a version, it doesn't gain us anything really.
@@ -3,6 +3,8 @@ | |||
* SPDX-License-Identifier: Apache-2.0 | |||
*/ | |||
allprojects { | |||
group = "aws.sdk.kotlin" |
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 it correct to have this aws.sdk.kotlin
if it's meant to be shared among all our projects?
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.
We aren't publishing this so it doesn't really matter. We have to choose some group though and the only groups we own are aws.smithy.kotlin
and aws.sdk.kotlin
. I don't care which it is but if you or anyone else has a preference thats fine.
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.
We have to choose some group though
Ok, this one should be fine
Issue #, if available:
n/a
Description of changes:
Create the
ktlint
andktlintFormat
tasks and include our customktlint-rules
by default. This works by adding our custom lint rules project as a runtime dependency of the plugin. The buildscript classpath is then included as the classpath for the ktlint exec task. This allows dowstream projects to define additional project specific rules should the need arise by simpling adding them to the buildscript classpathe.g.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.