Issue #2944: Changed the logging levels #3491
Conversation
…ents stated in DokkaLogging.kt
…ents stated in DokkaLogging.kt
…ents stated in DokkaLogging.kt
| @@ -15,5 +15,8 @@ | |||
| <mapping directory="$PROJECT_DIR$/dokka-integration-tests/gradle/projects/coroutines/kotlinx-coroutines" vcs="Git" /> | |||
| <mapping directory="$PROJECT_DIR$/dokka-integration-tests/gradle/projects/serialization/kotlinx-serialization" vcs="Git" /> | |||
| <mapping directory="$PROJECT_DIR$/dokka-integration-tests/maven/projects/biojava/biojava" vcs="Git" /> | |||
| <mapping directory="$PROJECT_DIR$/integration-tests/gradle/projects/coroutines/kotlinx-coroutines" vcs="Git" /> | |||
There was a problem hiding this comment.
could you please rebase on master? those VCS mappings are removed there (after some refactoring to integration tests)
| processSubmodules() | ||
|
|
||
| report("Finishing processing") | ||
| report("Finishing processing", LoggingLevel.PROGRESS) |
There was a problem hiding this comment.
may be it should be debug here?
| @@ -81,9 +93,9 @@ private fun timed(logger: DokkaLogger? = null, block: Timer.() -> Unit): Timer = | |||
| try { | |||
| block() | |||
| } catch (exit: GracefulGenerationExit) { | |||
| report("Exiting Generation: ${exit.reason}") | |||
| report("Exiting Generation: ${exit.reason}", LoggingLevel.PROGRESS) | |||
There was a problem hiding this comment.
may be here we can just do logger.progress(exit.reason) and leave report logic fully with debug level, and so no need to have report with custom logging level?
F.e:
try {
block()
report("Exiting Generation")
} catch (exit: GracefulGenerationExit) {
logger.progress(exit.reason)
report("Exiting Generation: ${exit.reason}")
} catch (cause: Throwable) {
report("Exiting Generation: ${cause.message}")
throw cause
}AFAIU report is mostly for debugging purpose
| @@ -164,7 +164,7 @@ internal fun topologicalSortByDependantSourceSets( | |||
| souceSet.dependentSourceSets.mapNotNull { dependentSourceSetId -> | |||
| sourceSets.find { it.sourceSetID == dependentSourceSetId } | |||
| // just skip | |||
| ?: null.also { logger.error("Unknown source set Id $dependentSourceSetId in dependencies of ${souceSet.sourceSetID}") } | |||
| ?: null.also { logger.warn("Unknown source set Id $dependentSourceSetId in dependencies of ${souceSet.sourceSetID}") } | |||
There was a problem hiding this comment.
I will mention it here, but it affects all error->warn changes.
Previously we emitted a lot of errors, but now we report only 3 of them and a lot of warnings. So if project will not set failOnWarning=true there could be a lot of warnings which could affect quality of generated HTML.
I would expect that users who generate documentation will not check that produced HTML is still fine after new declaration (or anything else) is added/removed. Also those warnings could be easily missed when updating project/Dokka, especially if the project documentation is build on CI.
I'm just trying to understand how it will affect our users. May be we need to set failOnWarning=true by default in this case, to prevent users from generation of bad HTML?
Or may be we should better specify what is successful documentation generation? As from DokkaLogger.error documentation:
This level is for logging error messages that describe what prevented Dokka from proceeding with successful documentation generation
What do we mean by successful documentation generation? Does HTML with absent data/declarations is considered successful?
Changed the logging levels according to the new requirements stated in DokkaLogging.kt