-
Notifications
You must be signed in to change notification settings - Fork 507
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 kotlin-logging as logging framework #1327
Merged
paul-dingemans
merged 7 commits into
pinterest:master
from
paul-dingemans:589-kotling-logging
Jan 29, 2022
Merged
Add kotlin-logging as logging framework #1327
paul-dingemans
merged 7 commits into
pinterest:master
from
paul-dingemans:589-kotling-logging
Jan 29, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactor output written to stdout / stderr to log statements where applicable. The kotlin-logging framework uses SLF4J as underlying logging framework. The latter framework does not allow to change the desired log level to be changed at runtime. As ktlint supports the command line flag "--debug" to print additional logging, it was needed to add logback as underlying logging framework. The IndentationRule and the RuleExtension class both changed the logging behavior based on the environment variable KTLINT_DEBUG. This variable could be assigned multiple values. This variable is now split into two distinct variables KTLINT_UNIT_TEST_TRACE and KTLINT_UNIT_TEST_DUMP_AST which can either be set to "on" to enable TRACE resp. DUMP_AST functionality or anything else to disable it. End users of ktlint should use the new command line flag "--trace" resp. the existing "--print-ast". Solved a bug in DumpASTRule when retrieval of the linenumber results in a IndexOutOfBoundsException cause by an AST mutation resulting in an invalid offset. In RuleExtension improved the logic regarding dumping the AST. When invoked from the extension, the DumpASTRule print the output to STDOUT instead of STDERR. Following needs explicit attention while reviewing: * Check the way the Gradle dependencies are added. I have little knowledge of Gradle. * I was not able to find published checksums for the added dependencies, so I have added the Gradle generated keys
2 tasks
# Conflicts: # gradle/verification-metadata.xml # ktlint-ruleset-standard/src/main/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRule.kt # ktlint/src/main/kotlin/com/pinterest/ktlint/Main.kt # ktlint/src/main/kotlin/com/pinterest/ktlint/internal/GitHookInstaller.kt # ktlint/src/main/kotlin/com/pinterest/ktlint/internal/PrintASTSubCommand.kt # ktlint/src/main/kotlin/com/pinterest/ktlint/internal/RuleSetsLoader.kt
…leset Note: The jar "ktlint-ruleset-experimental.jar" which previously was used as invalid jar in the unit test, was replaced in the master branch with a jar that actually does contain a ruleset provider. Now a new JAR (the "ktlint-reporter-html.jar") is added which does not contain a ruleset provider. Also the tests have been changed to check in the normal output for absence or existence of the warning.
…and what .editorconfig value are applicable from debug to trace Loglevel debug is enabled by the verbose mode which is commonly used on the CLI to detect which rule actually is throwing the lint error. This same flag is also used by the build step which checks the ktlint source code for violations. Due to way too many errors, it became too hard to find the files/lines which need to be changed.
# Conflicts: # ktlint-test/src/main/kotlin/com/pinterest/ktlint/test/RuleExtension.kt
FYI, as a consequence of this change, if any of these modules are used in a classpath with an existing logger (e.g. in an IDE), it will throw a api("com.pinterest.ktlint:ktlint-core:0.45.1") {
exclude("org.slf4j")
} Just a gotcha for any other ktlint library consumers 😁 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactor output written to stdout / stderr to log statements where applicable.
The kotlin-logging framework uses SLF4J as underlying logging framework. The latter framework does not allow to change the desired log level to be changed at runtime. As ktlint supports the command line flag "--debug" to print additional logging, it was needed to add logback as underlying logging framework.
The IndentationRule and the RuleExtension class both changed the logging behavior based on the environment variable KTLINT_DEBUG. This variable could be assigned multiple values. This variable is now split into two distinct variables KTLINT_UNIT_TEST_TRACE and KTLINT_UNIT_TEST_DUMP_AST which can either be set to "on" to enable TRACE resp. DUMP_AST functionality or anything else to disable it. End users of ktlint should use the new command line flag "--trace" resp. the existing "--print-ast".
Solved a bug in DumpASTRule when retrieval of the linenumber results in a IndexOutOfBoundsException cause by an AST mutation resulting in an invalid offset.
In RuleExtension improved the logic regarding dumping the AST. When invoked from the extension, the DumpASTRule print the output to STDOUT instead of STDERR.
Following needs explicit attention while reviewing:
Closes #589
Checklist
CHANGELOG.md
is updated