-
Notifications
You must be signed in to change notification settings - Fork 14
update update version shell #55
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
Conversation
WalkthroughThe pull request involves modifications to four shell scripts responsible for updating version numbers in various configuration files. These scripts ( Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (4)
update-ark-jdk8-version.sh (1)
Line range hint
11-12: Improve robustness of version replacementThe current implementation has several potential issues:
- It might fail silently if no matches are found
- The sed command might break with special characters in XML
- No backup of modified files is created
- The sed syntax might not work on all Unix systems
Consider this more robust implementation:
-git grep -rn "<sofa.ark.version>2\..*</sofa.ark.version>" | cat | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<sofa.ark.version\>2\..*\<\/sofa.ark.version\>/\<sofa.ark.version\>$1\<\/sofa.ark.version\>/g" {} +# Find files containing the version tag +FILES=$(git grep -l "<sofa.ark.version>2\." || true) + +if [ -z "$FILES" ]; then + echo "Warning: No files found containing sofa.ark.version tag" + exit 0 +fi + +# Process each file +echo "$FILES" | while read -r file; do + # Create backup + cp "$file" "$file.bak" + + # Update version using more reliable XML-aware sed + sed -i.tmp "s|<sofa\.ark\.version>2\.[^<]*</sofa\.ark\.version>|<sofa.ark.version>$DEST_VERSION</sofa.ark.version>|g" "$file" + + # Verify the change + if grep -q "<sofa.ark.version>$DEST_VERSION</sofa.ark.version>" "$file"; then + echo "Updated $file successfully" + rm "$file.tmp" + else + echo "Failed to update $file, restoring from backup" + mv "$file.bak" "$file" + rm -f "$file.tmp" + exit 1 + fi +doneupdate-ark-jdk17-version.sh (1)
Line range hint
1-12: Consider consolidating duplicate scriptsThis script is nearly identical to
update-ark-jdk8-version.sh, only differing in the version pattern (3.x vs 2.x). Consider consolidating these scripts to reduce duplication.Create a single script that accepts both version pattern and destination version:
-#!/bin/sh +#!/bin/bash + +# Usage: ./update-ark-version.sh <jdk_version> <dest_version> +# Example: ./update-ark-version.sh 17 2.3.0 -DEST_VERSION=$1 +JDK_VERSION=$1 +DEST_VERSION=$2 + +# Map JDK version to major version number +if [ "$JDK_VERSION" = "8" ]; then + VERSION_PREFIX="2" +elif [ "$JDK_VERSION" = "17" ]; then + VERSION_PREFIX="3" +else + echo "Error: JDK_VERSION must be either 8 or 17" + exit 1 +fi # 如果 DEST_VERSION 为空,则报错 -if [ -z "$DEST_VERSION" ]; then - echo "Error: DEST_VERSION must be provided." +if [ -z "$JDK_VERSION" ] || [ -z "$DEST_VERSION" ]; then + echo "Usage: $0 <jdk_version> <dest_version>" + echo "Example: $0 17 2.3.0" exit 1 fiThen apply the same robust version replacement logic as suggested for
update-ark-jdk8-version.sh, but using$VERSION_PREFIXinstead of hardcoded values.update-runtime-jdk17-version.sh (1)
Line range hint
11-12: Apply consistent improvements across all version update scriptsUntil a unified solution is implemented, this script needs the same robustness improvements suggested for the ark version scripts.
Apply the same improvements suggested for
update-ark-jdk8-version.sh, adjusting the tag name:
- Version format validation
- Proper error handling
- File backups
- Verification of changes
- Better sed patterns
Would you like me to provide the specific implementation?
update-runtime-jdk8-version.sh (1)
Line range hint
11-12: Fix critical issues in version update commandThe current implementation has several issues:
- The sed command is macOS-specific and will fail on Linux
- No error handling for git/sed operations
- Version pattern is too permissive
- No backup of modified files
Apply this fix for cross-platform compatibility and better error handling:
-# git grep -rn '2.2.12' | cat | awk -F: '{print $1}' | xargs -I {} sed -i '' 's/2.2.12/2.2.13/g' {} -git grep -rn "<koupleless.runtime.version>2\..*</koupleless.runtime.version>" | cat | awk -F: '{print $1}' | xargs -I {} sed -i '' "s/\<koupleless.runtime.version\>2\..*\<\/koupleless.runtime.version\>/\<koupleless.runtime.version\>$1\<\/koupleless.runtime.version\>/g" {} +# Create a function for platform-independent sed +sed_inplace() { + local file="$1" + local pattern="$2" + if [[ "$OSTYPE" == "darwin"* ]]; then + sed -i '' "$pattern" "$file" + else + sed -i "$pattern" "$file" + fi +} + +# Find files containing version tags +files=$(git grep -l "<koupleless.runtime.version>" || { echo "Error: Failed to search files"; exit 1; }) + +# Backup and update each file +for file in $files; do + # Create backup + cp "$file" "${file}.bak" || { echo "Error: Failed to create backup of $file"; exit 1; } + + # Update version + sed_inplace "$file" "s|<koupleless.runtime.version>[0-9]\+\.[0-9]\+\.[0-9]\+</koupleless.runtime.version>|<koupleless.runtime.version>$DEST_VERSION</koupleless.runtime.version>|g" || { + echo "Error: Failed to update $file" + mv "${file}.bak" "$file" # Restore from backup + exit 1 + } +done + +echo "Successfully updated version to $DEST_VERSION"This enhancement:
- Works on both macOS and Linux
- Creates backups before modification
- Provides proper error handling
- Uses a more specific version pattern
- Provides feedback on success
🧹 Nitpick comments (2)
update-runtime-jdk17-version.sh (1)
Line range hint
1-12: Consider a more flexible version management solutionThis script, along with its JDK8 counterpart and the ark version scripts, suggests a need for a more maintainable version management solution.
Consider these improvements:
Create a unified version management tool that:
- Reads version patterns from a configuration file
- Supports multiple version tags and JDK versions
- Provides proper validation and error handling
- Maintains an audit log of version changes
Example configuration structure:
version_patterns: ark: jdk8: tag: "sofa.ark.version" pattern: "2." jdk17: tag: "sofa.ark.version" pattern: "3." runtime: jdk8: tag: "koupleless.runtime.version" pattern: "2." jdk17: tag: "koupleless.runtime.version" pattern: "2."Would you like me to help create this unified solution?
update-runtime-jdk8-version.sh (1)
Line range hint
1-9: Enhance input validation and error handlingThe error handling could be improved by:
- Validating the version format
- Adding a usage message
- Checking for unexpected extra arguments
Consider applying this enhancement:
#!/bin/sh +usage() { + echo "Usage: $0 <version>" + echo "Example: $0 2.3.0" + exit 1 +} + DEST_VERSION=$1 +shift + +# Check for extra arguments +if [ $# -gt 0 ]; then + echo "Error: Unexpected extra arguments" + usage +fi # 如果 DEST_VERSION 为空,则报错 if [ -z "$DEST_VERSION" ]; then echo "Error: DEST_VERSION must be provided." - exit 1 + usage +fi + +# Validate version format (should match X.Y.Z) +if ! echo "$DEST_VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then + echo "Error: Version must be in X.Y.Z format" + usage fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
update-ark-jdk17-version.sh(1 hunks)update-ark-jdk8-version.sh(1 hunks)update-runtime-jdk17-version.sh(1 hunks)update-runtime-jdk8-version.sh(1 hunks)
🔇 Additional comments (1)
update-runtime-jdk8-version.sh (1)
Line range hint 1-12: Verify version patterns and affected files
Let's verify the current version patterns and potentially affected files in the codebase.
✅ Verification successful
Script update needs to handle both JDK8 and JDK17 version patterns
The verification shows that there are two distinct version patterns in use:
- JDK8 modules using versions like
1.4.0,2.1.9 - JDK17 modules using versions like
2.1.9,3.1.9
The script correctly handles updating the version patterns but should be used with caution as it affects multiple modules with different version requirements. The version update should be coordinated with the appropriate JDK version target.
Key affected files:
springboot1-samples/pom.xml: Using version 1.4.0springboot3-samples/pom.xml: Using version 2.1.9- Various module pom.xml files inheriting these versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze version patterns and affected files
echo "=== Files containing koupleless.runtime.version ==="
rg -l "<koupleless.runtime.version>"
echo -e "\n=== Current version patterns ==="
rg -o "<koupleless.runtime.version>[^<]+</koupleless.runtime.version>"
echo -e "\n=== Checking for other version tags that might need updating ==="
rg -o "<[^>]*version>[^<]+</[^>]*version>"
echo -e "\n=== Checking for version numbers in other contexts ==="
rg -o "\b\d+\.\d+\.\d+\b"
Length of output: 68946
| echo "Error: DEST_VERSION must be provided." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add version format validation
While the error message is clearer now, the script should validate that DEST_VERSION follows the expected format (e.g., starts with "2.") before proceeding with replacements.
if [ -z "$DEST_VERSION" ]; then
echo "Error: DEST_VERSION must be provided."
exit 1
+elif ! echo "$DEST_VERSION" | grep -qE '^2\.[0-9]+\.[0-9]+(-SNAPSHOT)?$'; then
+ echo "Error: DEST_VERSION must match pattern 2.x.y[-SNAPSHOT]"
+ exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo "Error: DEST_VERSION must be provided." | |
| exit 1 | |
| fi | |
| if [ -z "$DEST_VERSION" ]; then | |
| echo "Error: DEST_VERSION must be provided." | |
| exit 1 | |
| elif ! echo "$DEST_VERSION" | grep -qE '^2\.[0-9]+\.[0-9]+(-SNAPSHOT)?$'; then | |
| echo "Error: DEST_VERSION must match pattern 2.x.y[-SNAPSHOT]" | |
| exit 1 | |
| fi |
Summary by CodeRabbit
New Features
DEST_VERSION.Bug Fixes
DEST_VERSIONinputs across all scripts.