docs: fix comment-rot, README Quick-Start scope, and benchmark inaccuracies#56
Merged
Conversation
…racies Outcome of a repo-wide documentation audit. All changes are comment/KDoc, test-display-name, or markdown only — zero runtime behaviour change. Comment-rot / stale-reference / misleading (code was correct, comments fixed): - DeliveryInfo: serialize() rationale claimed CompositeMessageDeliverer routes by deserializing metadata; it routes via OutboxEntry.deliveryType. - TransactionRunner: KDoc named non-existent modules okapi-spring / okapi-ktor -> okapi-spring-boot / okapi-exposed. - TransactionContextValidator: dropped false "Standalone: no-op (always true)" (no such impl ships) -> "user-provided implementation". - ExposedConnectionProvider: current() -> currentOrNull() to match the code. - ConnectionLeakProofTest: removed false history of an "old getConnection() contract" that never existed; describes the design pitfall instead. - MultiDataSourceTransactionTest: corrected Spring semantics — REQUIRED joins the existing tx (no savepoint); savepoints are PROPAGATION_NESTED. - MicrometerOutboxMetricsTest: renamed/retitled to match what it asserts (lazy refresh, no store query on construction), not "eager NaN" framing. - OkapiLiquibaseAutoConfiguration: @AutoConfigureAfter -> @autoConfiguration(after=). - OkapiMetricsProperties: refresh transaction is conditional on a tx manager. - OutboxAutoConfiguration: TransactionRunner requirement is conditional on a scheduler being enabled (publish-only mode needs none). - LiquibaseAutoConfigurationTest: dropped brittle build.gradle.kts:35-36 line ref. - OutboxAutoConfigurationTransactionRunnerTest: dropped stale "currently is silent" from a test name (the assertion requires the WARN to be emitted). - ObservabilityEndToEndTest: removed a comment that just narrated the code. README: trimmed the FOR UPDATE SKIP LOCKED internals out of Quick Start to a one-liner + pointer; moved the mechanism, multi-DataSource, TransactionTemplate precedence, and direct-scheduler-construction paragraphs into a new "Advanced: transactions & multi-DataSource" section (content preserved verbatim). benchmarks/README: heap 2g -> 8g (matches build.gradle.kts; 2g OOMs in throughput mode); replaced the wrong "2 transports x 3 x 8" wall-time formula with an accurate matrix description; fixed an untranslated word.
The inline comment was fixed to explain PROPAGATION_REQUIRED joins the existing transaction (no savepoint), but the test display name still read '(savepoint)', contradicting the comment and perpetuating the misconception the audit removed. (Code review follow-up.)
This file contains hidden or 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
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.
Summary
Outcome of a repo-wide documentation audit (cross-checking every comment/KDoc against the code it describes). All changes are comment/KDoc, test-display-name, or markdown only — zero runtime behaviour change. Verified:
./gradlew ktlintCheckgreen,:okapi-core:test+:okapi-micrometer:testpass, all edited test sources compile.Each fix was independently confirmed by an adversarial verifier that decided which side was correct — in every case here the code was right and the documentation was wrong (one separate genuine code bug found by the same audit is tracked separately, not in this PR).
Comment-rot / stale-reference / misleading
DeliveryInfo.ktCompositeMessageDelivererroutes by deserializing metadataOutboxEntry.deliveryType; serialize() is for the concrete deliverer's own deserializeTransactionRunner.ktokapi-spring/okapi-ktorokapi-spring-boot/okapi-exposedTransactionContextValidator.ktExposedConnectionProvider.ktTransactionManager.current()currentOrNull()(matches code)ConnectionLeakProofTest.ktgetConnection(): Connectioncontract" that never existedMultiDataSourceTransactionTest.ktMicrometerOutboxMetricsTest.ktOkapiLiquibaseAutoConfiguration.kt@AutoConfigureAfter(×2)@AutoConfiguration(after = ...)OkapiMetricsProperties.ktOutboxAutoConfiguration.ktLiquibaseAutoConfigurationTest.ktbuild.gradle.kts:35-36ref (pointed at unrelated block)OutboxAutoConfigurationTransactionRunnerTest.ktObservabilityEndToEndTest.ktREADME — Quick Start scope
The Quick Start had drifted into reference-manual depth. Trimmed the
FOR UPDATE SKIP LOCKEDmechanism explanation to a one-liner + pointer, and moved (verbatim, nothing deleted) the mechanism paragraph, the multi-DataSource paragraph, theTransactionTemplate-precedence paragraph, and the direct-scheduler-construction block into a newAdvanced: transactions & multi-DataSourcesection placed right before How It Works.benchmarks/README
-Xms2g -Xmx2g→-Xms8g -Xmx8g(matchesbuild.gradle.kts; 2g OOMs in throughput mode).httpLatencyMs; warmup/measurement/fork were conflated) with an accurate matrix description.Test plan
./gradlew ktlintCheck— green./gradlew :okapi-core:test :okapi-micrometer:test— pass:okapi-spring-boot:compileTestKotlin+:okapi-integration-tests:compileTestKotlin— compile clean (container E2E tests need Docker, unaffected — comment-only edits)