fix log rolling update into wrong date #1004
Conversation
WalkthroughThe changes involve downgrading the Changes
Poem
Tip Announcements
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- sofa-ark-bom/pom.xml (1 hunks)
- sofa-ark-parent/core-impl/archive/src/main/java/com/alipay/sofa/ark/loader/DirectoryContainerArchive.java (1 hunks)
- sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/ArkContainer.java (3 hunks)
Additional comments not posted (3)
sofa-ark-bom/pom.xml (1)
17-17: Verify the impact of downgrading the log.sofa.starter version.The version of
log.sofa.starterhas been downgraded from3.16.3to3.2.0. It's crucial to verify if this change reintroduces any previously fixed bugs or compatibility issues.sofa-ark-parent/core-impl/archive/src/main/java/com/alipay/sofa/ark/loader/DirectoryContainerArchive.java (1)
45-45: Question the rationale behind downgrading logging libraries.The downgrade of
logback-coreandlogback-classicfrom1.2.13to1.2.9could potentially reintroduce bugs or compatibility issues that were addressed in later versions. Please provide the reasoning behind this change and ensure that it has been thoroughly tested, especially in terms of logging functionality and system stability.sofa-ark-parent/core-impl/container/src/main/java/com/alipay/sofa/ark/container/ArkContainer.java (1)
251-272: Approve the streamlined logging configuration logic but verify complete functionality.The changes to
reInitializeArkLoggermethod introduce a more modular approach to managing logging configurations. However, please ensure that all necessary logging configurations are correctly applied and that no essential configurations are omitted in the process. It would be beneficial to conduct thorough testing to verify that the new logic behaves as expected across different scenarios and configurations.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1004 +/- ##
============================================
+ Coverage 78.12% 78.30% +0.17%
+ Complexity 875 873 -2
============================================
Files 170 170
Lines 7030 7033 +3
Branches 1039 1037 -2
============================================
+ Hits 5492 5507 +15
+ Misses 1004 994 -10
+ Partials 534 532 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pom.xml (1 hunks)
Additional comments not posted (2)
pom.xml (2)
17-17: Typographical error in property name.The property name
maven.gpg.pluignshould be corrected tomaven.gpg.pluginto avoid configuration errors.Apply this diff to correct the typo:
-<maven.gpg.pluign>1.6</maven.gpg.pluign> +<maven.gpg.plugin>1.6</maven.gpg.plugin>Likely invalid or redundant comment.
17-17: Approved version change to snapshot.The change to
2.2.14-SNAPSHOTis appropriate for a development phase. Ensure that all project dependencies and modules are compatible with this snapshot version.Run the following script to verify compatibility with the new snapshot version:
Verification successful
Version change to 2.2.14-SNAPSHOT verified across modules.
The version placeholder
${sofa.ark.version}is consistently used across multiple modules, indicating a coordinated update. Ensure compatibility during testing phases.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of all modules and dependencies with the new snapshot version. # Test: Search for any incompatible version specifications. Expect: No incompatible versions found. rg --type xml -A 5 $'<version>${sofa.ark.version}</version>'Length of output: 193
Script:
#!/bin/bash # Description: Verify compatibility of all modules and dependencies with the new snapshot version. # Test: Search for any XML files referencing the sofa.ark.version placeholder. rg --type xml -F '<version>${sofa.ark.version}</version>'Length of output: 2426
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java (2 hunks)
Files skipped from review due to trivial changes (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java
lvjing2
force-pushed
the
youji-dev
branch
2 times, most recently
from
b13a267
to
df552e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java
lvjing2
force-pushed
the
youji-dev
branch
2 times, most recently
from
cd5c480
to
ef05795
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- sofa-ark-parent/support/ark-springboot-integration/ark-springboot-starter/src/test/java/com/alipay/sofa/ark/springboot/web/ArkTomcatServletWebServerFactoryTest.java
This reverts commit 75fa690
Summary by CodeRabbit
New Features
Bug Fixes
Refactor