Remove slf4j-simple dependency#173
Conversation
Removed slf4j-simple dependency from pom.xml. Libraries should only have a dependency on slf4j-api and not on slf4j-simple or any other specific logging binding (like logback-classic or log4j-core). This allows the application that uses your library to choose its own logging implementation, which is the main purpose of the SLF4J facade.
Summary of ChangesHello @gwttk, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the project's dependency management by removing the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly removes the slf4j-simple dependency, which is a best practice for Java libraries to avoid forcing a specific logging implementation on consumers. This allows applications using this SDK to choose their own logging backend. I've included a suggestion to add a logging implementation back but scoped for tests, which will help with debugging during development.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Libraries should only depend on slf4j-api, not bind a specific logging implementation. Keep slf4j-simple available for tests only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the contribution! This change has been included in v2.22.21 (commit 86e2fed). Closing this PR. |
* Fix VideoSynthesis ignoring custom baseUrl for asyncCall/call requests (#199) The baseUrl parameter was stored but never passed to ApiServiceOption, causing task creation to always use the default mainland China endpoint. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add code and message fields to ImageGenerationOutput (#187) Error messages from wan2.6 image generation were silently dropped because the output class lacked these fields. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix Recognition NPE that silently kills the ASR stream (#161) Two null-safety issues could cause NPE during stream processing: 1. RecognitionResult.fromDashScopeResult: output could be null when the WebSocket message has no payload, causing NPE on cast and terminating the entire RxJava stream — all subsequent ASR results would be silently dropped. 2. Recognition.call onEvent: getSentence() could be null but was dereferenced without null check before calling isHeartbeat(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix SSE EventSource connection leak on stream disposal (#37) The Flowable-based streamOut created an EventSource without storing a reference or setting a cancellable on the emitter. When subscribers disposed the Flowable, the EventSource kept running and the HTTP connection was never released. Now the EventSource is stored, set as cancellable on the emitter, and the close() method properly cancels it. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add cache_creation_input_tokens to MultiModalConversationTokensDetails (#214) The field was missing, causing cache token usage from context caching to be silently dropped for multimodal conversations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add json_schema support to ResponseFormat (#172) ResponseFormat now supports type "json_schema" with a nested JsonSchemaFormat containing name, strict, and schema fields, consistent with the OpenAI-compatible structured output API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add HappyHorse video generation model constant (#210) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Update README quickstart examples to use Generation API (#166) Replace deprecated Conversation/QWenConversationParam with Generation/GenerationParam. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix rxjava version incorrectly changed from 2.2.21 to 2.22.21 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add parentNodeId to WorkflowMessage and update version to 2.22.21 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix code formatting in RecognitionResult.java Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix thread-safety issue with concurrent SSE streams in OkHttpHttpClient (#37) Replace single activeEventSource with ConcurrentHashMap-backed Set to support multiple concurrent streaming requests safely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove samples from jar packaging (#226) Remove build-helper-maven-plugin config that added samples/ as a source directory, causing package-less sample classes to pollute classpath root. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Change lombok dependency scope from compile to provided (#198) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Change slf4j-simple dependency scope to test (#173) Libraries should only depend on slf4j-api, not bind a specific logging implementation. Keep slf4j-simple available for tests only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix race condition, task shadowing, and redundant annotation - OkHttpHttpClient: use AtomicBoolean to prevent EventSource leak when onFailure/onClosed fires before add() - VideoSynthesis: assign task parameter to this.task in constructor - ResponseFormat: remove redundant @Builder.Default on Boolean strict Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove unused Builder import from ResponseFormat Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert parentNodeId field from WorkflowMessage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: zhansheng.lzs <zhansheng.lzs@alibaba-inc.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Removed slf4j-simple dependency from pom.xml.
Libraries should only have a dependency on slf4j-api and not on slf4j-simple or any other specific logging binding (like logback-classic or log4j-core). This allows the application that uses your library to choose its own logging implementation, which is the main purpose of the SLF4J facade.