Address RC feedback #292
Merged
Address RC feedback #292
Conversation
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
Removes jackson-databind dependency from servlet module and dynlog. It remains in the dynlog module as transitive dependency fro java-jwt. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
Replace StringUtils.isBlank by custom implementations. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
Switch to <release> tag with compiler plugin. This forces Java 11 compliant code requiring a test change. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
The integration is compatible with any servlet API version so far. This helps integration with older servlet containers increasing compatibility. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
j-denner
reviewed
Nov 4, 2025
There was a problem hiding this comment.
In general I recommend using the oldest supported API (which you moved to with Servlet downgrade to 6.0.0)
Please also have a look at my refactoring example regarding Immutable list with List.of() this is a cross topic and potentially beyond the scope of this PR.
Not approving right now since there is still ongoing work from your side.
...a-logging-support-core/src/main/java/com/sap/hcp/cf/logging/common/converter/LineWriter.java
Show resolved
Hide resolved
.../com/sap/hcf/cf/logging/opentelemetry/agent/ext/binding/CloudFoundryServicesAdapterTest.java
Outdated
Show resolved
Hide resolved
This version is compatible with Spring Boot 3.5.7. There should be no issues with running different versions of that dependency. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
Adds the maven-enforcer-plugin to ensure build with Java 17. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
j-denner
reviewed
Nov 4, 2025
j-denner
approved these changes
Nov 4, 2025
Replaces `Arrays.asList` and `Collections.singletonList` with `List.of`. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
KarstenSchnitter
force-pushed
the
rc-comments
branch
from
fb1611d to
5119152
Compare
Co-authored-by: j-denner <53571068+j-denner@users.noreply.github.com>
|
This should address all feedback so far. I think this is now Ready for review. |
vtintillier
reviewed
Nov 4, 2025
| @Override | ||
| public void write(String str, int off, int len) { | ||
| if (StringUtils.isNotBlank(str)) { | ||
| if (str != null && !str.isBlank()) { |
There was a problem hiding this comment.
any reason for not providing a shared method in support-core module to be used everywhere?
There was a problem hiding this comment.
It's too small of a functionality to do that. Using the StringUtils for it was already overkill.
Provide clear separation of dependency versions: * runtime (excl. agent extension and sample app) * test * Maven plugins Align runtime dependencies with latest Spring Boot version Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
KarstenSchnitter
force-pushed
the
rc-comments
branch
from
e43fdff to
670ea92
Compare
Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
Now that Jackson is downgraded, the upgrade of java-jwt creates no more issues. Signed-off-by: Karsten Schnitter <k.schnitter@sap.com>
vtintillier
approved these changes
Nov 4, 2025
Same from my side. |
j-denner
approved these changes
Nov 4, 2025
|
Thanks for all the help. |
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.
Addressing comments on the release candidate: