terrymanu commented on PR #38744:
URL: https://github.com/apache/shardingsphere/pull/38744#issuecomment-4561347928
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** PR #38744 latest head
`8de6a3d15938e1307c6710ef865dc2f4cba42589`; base `master` at
`72f436c6ea6637fb9f8648265c62c745041b61a0`; local merge-base
`72f436c6ea6637fb9f8648265c62c745041b61a0`. Reviewed all 7 GitHub
`/pulls/38744/files` entries, and the local triple-dot diff file list matched
GitHub exactly:
-
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/capability/tool/MCPToolClarificationPolicy.java`
-
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/validator/ShardingSphereServerTransportSecurityValidator.java`
-
`mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/completion/provider/MetadataCompletionProvider.java`
-
`mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/resource/handler/metadata/MetadataResourceHandler.java`
-
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/descriptor/MCPResourceNavigationPayloadBuilder.java`
-
`mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/descriptor/MCPDescriptorCatalogIndexTest.java`
-
`test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/llm/conversation/LLMMCPConversationRunner.java`
- **Not Reviewed Scope:** GitHub Actions / CI status was not reviewed. No
full MCP runtime smoke test was performed because the patch is a
lambda-parameter rename only and does not change control flow or runtime
behavior.
- **Need Expert Review:** No dedicated security, concurrency, SQL parser, or
performance expert review is required for this change.
### Basis
- The PR consistently addresses its stated goal, "Standardize Optional
lambda parameter names", by renaming Optional-chain lambda parameters to
`optional` in the affected MCP and MCP e2e files.
- The diff is limited to local lambda parameter names. There are no method
signature changes, public API/SPI changes, configuration changes, SQL parser
changes, routing/name-resolution changes, fallback-precedence changes, or
dependency changes.
- The touched shared MCP paths were checked for blast radius:
- MCP tool elicitation clarification policy
- HTTP transport security header lookup
- metadata completion default-schema inference
- metadata resource navigation payload assembly
- descriptor navigation payload assembly
- MCP descriptor catalog index test
- LLM MCP e2e tool-call validation
- For the non-target/shared-path counterexample review, the HTTP transport
validator and metadata resource/navigation paths still execute the same
stream/Optional chains and use the same values; only the lambda parameter
identifier changed.
- Config-disabled / feature-flag-off review: no feature flags or config
branches are changed by this PR.
- Adversarial pass: I looked for cross-feature runtime behavior changes,
partial root-cause coverage, and adjacent-path regressions. The change remains
behavior-neutral because all edited expressions keep the same receiver,
predicates, mapped values, and terminal operations.
### Verification
- PR boundary verification:
- `git fetch apache master pull/38744/head:refs/remotes/apache/pr/38744`
- `git merge-base refs/remotes/apache/master refs/remotes/apache/pr/38744`
- `git diff --name-status <merge-base>..refs/remotes/apache/pr/38744`
- Local file list matched GitHub `/pulls/38744/files` exactly.
- Reviewer-run local checks on this latest PR head:
- `./mvnw spotless:apply -Pcheck -T1C` passed with exit code 0.
- `./mvnw checkstyle:check -Pcheck -T1C` passed with exit code 0.
- `./mvnw -pl mcp,test/e2e/mcp -am -DskipITs -Dspotless.skip=true
-Dcheckstyle.skip=true -DskipTests test-compile` passed with exit code 0.
- A focused scan for remaining Optional `map/filter/ifPresent` lambda
parameters in `mcp` and `test/e2e/mcp` did not find remaining candidates that
matched this PR's rename pattern.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
