Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove bcprov-jdk15on as it isn't used #2169

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

vitalijr2
Copy link
Collaborator

@vitalijr2 vitalijr2 commented Sep 14, 2023

  • There is an issue with bcprov-jdk15on. Of course we can increase its version to 1.76. But I checked: only the benchmark module depends on it and that module doesn't use any classes from BC.
  • The benchmark module has the dependency issue: rxnetty-common depends on SLF4J 1.7.6 but now we use SLF4J 2.0* that is not binary compatible with previous branch.
    [INFO] +- io.reactivex:rxnetty-common:jar:0.5.3:compile
    [INFO] |  \- org.slf4j:slf4j-api:jar:1.7.6:compile
    

This fix is not major on my opinion and can be added to any further version.

Summary by CodeRabbit


  • Refactor: Removed the dependency on the Bouncy Castle library (bcprov-jdk15on). This change simplifies our dependencies and may affect some cryptographic features, depending on their usage in the project.
  • New Feature: Added dependencies on slf4j-api and slf4j-nop, specifying the version of slf4j. This introduces a new logging framework to the project, improving the visibility and traceability of events within the application.

@coderabbitai
Copy link

coderabbitai bot commented Sep 14, 2023

Walkthrough

The changes primarily revolve around the removal of the Bouncy Castle library dependency and the addition of SLF4J for logging. This could impact cryptographic functionality and alters the project's logging mechanism.

Changes

File Summary
benchmark/pom.xml Removed bcprov-jdk15on, added slf4j-api and slf4j-nop. Specified slf4j version.
pom.xml Removed Bouncy Castle library (bcprov-jdk15on). This may affect security or cryptographic features.

🐇💻

"In the land of code, where logic is king,

Dependencies come and dependencies go, it's a normal thing.

Farewell to Bouncy Castle, its reign was fun,

Welcome SLF4J, a new era has begun.

With every pull request, we hop and we leap,

In the world of software, there's no time for sleep!" 🌙🚀


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • Mention @coderabbitai in any review comment for bot assistance.
  • Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

  • Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Commits Files that changed from the base of the PR and between b04bf9a and 8036801 commits.
Files selected for processing (2)
  • benchmark/pom.xml (2 hunks)
  • pom.xml (2 hunks)
Files skipped from review due to trivial changes (1)
  • benchmark/pom.xml
Additional comments (Suppressed): 2
pom.xml (2)
  • 87-92: The removal of the Bouncy Castle library version property (bouncy.version) is consistent with the PR summary. Ensure that this library was indeed not used anywhere in the project, as its removal could impact cryptographic functionality if it was being utilized.

  • 340-345: The Bouncy Castle dependency has been removed as per the PR summary. As mentioned earlier, verify that this library was not used anywhere in the project to avoid potential issues with cryptographic functionality.

@velo velo merged commit 6b7dc4f into OpenFeign:master Sep 14, 2023
1 check passed
@vitalijr2 vitalijr2 deleted the bcprov-is-not-used branch February 21, 2024 10:57
velo pushed a commit that referenced this pull request Oct 7, 2024
velo pushed a commit that referenced this pull request Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants