Maintenance for 5.2.0 - #3117
Open
afranken wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Documentation maintenance to align S3Mock’s published guidance with current Spring Boot 4.x / Kotlin compatibility targets and with the actual environment-variable names used by the Testcontainers harness.
Changes:
- Updated Testcontainers harness docs to use underscore-separated env var names (matching
S3MockContainerconstants) and clarified config derivation guidance. - Removed hard-coded “Kotlin 2.3+ / Spring Boot 4.0.x” phrasing in favor of “Kotlin (API/lang 2.2) / Spring Boot 4.x” where appropriate, pointing readers to
pom.xmlfor pinned versions. - Corrected architecture doc paths to reflect current
s3/*package structure.
Review Findings
Should fix
testsupport/testcontainers/AGENTS.md:31,56— The wording currently implies relaxed binding requires underscore-splitting camelCase boundaries. The linked Spring Boot relaxed binding rules are more permissive; underscore splitting can be documented as a project convention for readability instead. (Source: Spring Boot relaxed binding reference linked from the file.)
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testsupport/testcontainers/AGENTS.md | Updates env var naming guidance for the Testcontainers harness (underscore-separated) |
| testsupport/common/AGENTS.md | Aligns referenced env var example with underscore-separated form |
| README.md | Updates technology/version wording; avoids restating pinned toolchain versions |
| docs/KOTLIN.md | Clarifies Kotlin compatibility targets vs pinned compiler/toolchain versions |
| docs/DECISIONS.md | Generalizes Spring Boot/Kotlin version statements to 4.x / pinned-in-pom wording |
| docs/ARCHITECTURE.md | Fixes layer path references to s3/* package structure |
| AGENTS.md | Updates root tech stack wording and fixes docs table punctuation |
Suppressed comments (1)
testsupport/testcontainers/AGENTS.md:56
- This repeats the same idea as above and currently frames underscore-splitting as a strict derivation rule. If the intent is to document the project's convention (matching the constants used in S3MockContainer), clarify it's a preferred naming style rather than a requirement of relaxed binding.
When a new property is added to `StoreProperties` or `ControllerProperties` in `server/`, derive the env var key by uppercasing the Spring property name, replacing `.` with `_`, and splitting each camelCase word boundary with `_` (e.g. `initialBuckets` → `INITIAL_BUCKETS`):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Property Name Format | ||
|
|
||
| S3MockContainer configures the container via Docker environment variables. Spring Boot inside the container automatically maps env vars to properties using its [relaxed binding](https://docs.spring.io/spring-boot/reference/features/external-config.html#features.external-config.typesafe-configuration-properties.relaxed-binding.environment-variables) rules: uppercase the Spring property key and replace `.` with `_`. | ||
| S3MockContainer configures the container via Docker environment variables. Spring Boot inside the container automatically maps env vars to properties using its [relaxed binding](https://docs.spring.io/spring-boot/reference/features/external-config.html#features.external-config.typesafe-configuration-properties.relaxed-binding.environment-variables) rules: uppercase the Spring property key, replace `.` with `_`, and split each camelCase word boundary with `_`. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
CHANGELOG.md:205
- The version bumps documented here don’t match the versions currently pinned in the root pom.xml (spotless-maven-plugin is 3.8.0 and checkstyle is 13.8.0). Either update pom.xml to the stated versions or adjust these release notes so they reflect the actual dependency versions being shipped.
* Bump com.diffplug.spotless:spotless-maven-plugin from 3.8.0 to 3.9.0
* Bump com.puppycrawl.tools:checkstyle from 13.6.0 to 13.9.0
CHANGELOG.md:210
- This actions/stale bump doesn’t match the workflow configuration: .github/workflows/stale.yml is pinned to v10.4.0. Update this entry to match what’s actually used (or bump the workflow if 11.0.0 is intended).
* Bump actions/stale from 10.3.0 to 11.0.0
afranken
force-pushed
the
chore/maintenance-for-5.2.0-ctd-3
branch
from
August 5, 2026 09:44
63af643 to
a09b7e0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (4)
testsupport/testcontainers/AGENTS.md:31
- Spring Boot relaxed binding does not require inserting underscores at camelCase boundaries; it treats underscores as separators and will bind both forms. This sentence currently states the underscore split as a binding rule, which is misleading. Consider describing the underscore split as this module’s convention for readability, while noting Spring will accept both.
S3MockContainer configures the container via Docker environment variables. Spring Boot inside the container automatically maps env vars to properties using its [relaxed binding](https://docs.spring.io/spring-boot/reference/features/external-config.html#features.external-config.typesafe-configuration-properties.relaxed-binding.environment-variables) rules: uppercase the Spring property key, replace `.` with `_`, and split each camelCase word boundary with `_`.
testsupport/testcontainers/AGENTS.md:56
- Same issue here: the text frames camelCase underscore splitting as part of Spring’s relaxed binding rules rather than a convention. Rephrasing this as a convention (while keeping the example) would make the guidance accurate.
When a new property is added to `StoreProperties` or `ControllerProperties` in `server/`, derive the env var key by uppercasing the Spring property name, replacing `.` with `_`, and splitting each camelCase word boundary with `_` (e.g. `initialBuckets` → `INITIAL_BUCKETS`):
CHANGELOG.md:203
- These CHANGELOG entries claim dependency version bumps that are not reflected in the current build configuration (root pom.xml still pins aws-v2.version=2.49.3, aws-kotlin.version=1.8.14, kotlin.version=2.4.0, spotless-maven-plugin.version=3.8.0, checkstyle.version=13.8.0, xmlunit-assertj3.version=2.12.0). Please either include the corresponding version bumps in the build files, or adjust the CHANGELOG to match what was actually changed in this PR.
* Version updates (deliverable dependencies)
* Bump software.amazon.awssdk:bom from 2.46.11 to 2.50.1
* Bump aws.sdk.kotlin:s3-jvm from 1.6.96 to 1.8.22
* Bump kotlin.version from 2.3.21 to 2.4.10
* Version updates (build dependencies)
CHANGELOG.md:214
- These CHANGELOG entries claim GitHub Action bumps (setup-java 5.7.0, stale 11.0.0, codeql-action 4.37.4), but the workflows under .github/workflows still reference setup-java v5.6.0, actions/stale v10.4.0, and github/codeql-action v4.37.3. Please either update the workflow pins or correct these CHANGELOG lines.
* Bump actions/checkout from 7.0.0 to 7.0.1
* Bump actions/setup-java from 5.3.0 to 5.7.0
* Bump actions/stale from 10.3.0 to 11.0.0
* Bump docker/setup-qemu-action from 4.1.0 to 4.2.0
* Bump github/codeql-action from 4.36.2 to 4.37.4
* Bump ossf/scorecard-action from 2.4.3 to 2.4.4
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.
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: