gnodet commented on PR #1762: URL: https://github.com/apache/maven-resolver/pull/1762#issuecomment-4961747971
Thanks for the thorough rework, @kwin! The new commit addresses all 5 findings from my earlier review: 1. ✅ `transportProperties` field is now `private final` with `emptyMap()` default — immutability contract restored 2. ✅ `HttpTransportPropertiesBuilder.build()` returns `unmodifiableMap` — no more mutable reference leak 3. ✅ Apache transporter now reports transport properties on all task types (Get, Peek, Put) via a dedicated `HttpRequestExecutor` 4. ✅ The new `transportPropertiesAvailable()` method is a clean design — it decouples property notification from the `transportStarted()` transfer lifecycle, solving the PeekTask inconsistency without overloading the event semantics 5. ✅ Dead `SSL_1_0` / `SSL_2_0` enum values removed The `transportPropertiesAvailable()` approach is a nice architectural improvement over the original design — separating concerns between "transfer is starting" and "transport metadata is available" makes the API much clearer. --- *🤖 This follow-up was generated by [ForgeBot](https://github.com/gnodet/forgebot-skills). Reviewing new commits on a previously reviewed PR.* -- 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]
