Conversation
…he#427) * chore: update release workflow and .gitignore entries Bumped default release version to 1.7.0 and updated default GPG user in the release workflow. Added installation step for subversion on Ubuntu. Appended WARP.md to .gitignore. * refactor: unify release validation script and add local path support Deleted validate-release-in-local.sh and enhanced validate-release.sh to support both SVN and local directory validation. Added color-coded output, improved argument handling, and included Java version checks for better usability and error reporting. * refactor: revamp release validation script with enhanced checks V2 Major rewrite of validate-release.sh for Apache HugeGraph, adding modular structure, improved logging, error/warning collection, colorized output, and comprehensive validation steps for source and binary packages. New features include dependency checks, GPG key management, license compliance, file size and binary checks, version consistency, and automated server/toolchain testing. Usage instructions and help output are expanded for clarity. * refactor: enhance release validation for multi-arch and license checks V3 Updated CI workflow to support additional OS and architectures (arm64, macOS 14). Improved documentation and script usage instructions. The license header check now covers more file types and excludes generated/vendor files. Maven build commands in docs and scripts now use '-DskipTests' and '-Dcheckstyle.skip=true' for consistency. Added a detailed README for the release validation script. * refactor: improve validation script error context and reporting V4 Adds contextual error and warning reporting with step and package information, enhances license category and header checks, improves version consistency logic, and refines summary output with execution time and clearer formatting. These changes make validation results more actionable and easier to interpret, especially for multi-package and multi-step validations. * fix: add JSON to CATEGORY_X license validation The JSON license was added to the CATEGORY_X regex in the binary package validation step to ensure packages with this license are properly flagged during release validation. * Update .github/workflows/validate-release.yml * Update .github/workflows/validate-release.yml --------- Co-authored-by: Peng Junzhi <78788603+Pengzna@users.noreply.github.com>
|
@codecov-ai-reviewer review |
|
Caution Review failedThe pull request is closed. 概览本次提交引入了新的发布版本验证工作流，更新了现有工作流的配置和输入参数，修订了文档中的Maven测试命令语义，以及在.gitignore中添加了新的忽略文件条目。 变更内容
预期代码审查工作量🎯 3 (中等) | ⏱️ ~25-30 分钟 特别需要关注的区域：
可能相关的PR
诗歌
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Pengzna, 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 significantly upgrades the release validation process for Apache HugeGraph by introducing a robust, automated shell script. The new script streamlines the verification of release packages, ensuring compliance with Apache policies and functional correctness through a series of checks, from cryptographic signatures to runtime tests. Accompanying documentation has been added and existing guidelines updated to support this enhanced validation workflow. Highlights
Ignored Files
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 is an excellent and comprehensive refactoring of the release validation script. The new version is much more robust, user-friendly, and maintainable. The structured approach with functions, detailed logging, colored output, and the 'collect and report' error handling strategy are all significant improvements. The new README is also very detailed and helpful. I've made a few suggestions to further improve robustness and usability, mainly around Java version detection, argument parsing, and handling of file search results. Overall, great work on this major enhancement!
| check_java_version() { | ||
| local required_version=$1 | ||
|
|
||
| info "Checking Java version..." | ||
|
|
||
| if ! command -v java &> /dev/null; then | ||
| collect_error "Java is not installed or not in PATH" | ||
| return 1 | ||
| fi | ||
|
|
||
| local current_version=$(java -version 2>&1 | head -n 1 | awk -F '"' '{print $2}' | awk -F '.' '{print $1}') | ||
| info "Current Java version: $current_version (Required: ${required_version})" | ||
|
|
||
| if [[ "$current_version" != "$required_version" ]]; then | ||
| collect_error "Java version mismatch! Current: Java $current_version, Required: Java ${required_version}" | ||
| collect_error "Please switch to Java ${required_version} before running this script" | ||
| return 1 | ||
| fi | ||
|
|
||
| success "Java version check passed: Java $current_version" | ||
| mark_check_passed | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
The current method for parsing the Java version is not robust and will fail for Java 8. java -version for Java 8 outputs a string like java version "1.8.0_341", and your current awk command would incorrectly parse this as version 1. This would cause the validation to fail incorrectly on a system with Java 8, even if Java 8 were the required version. The suggested change handles both Java 8 and Java 9+ version string formats.
check_java_version() {
local required_version=$1
info "Checking Java version..."
if ! command -v java &> /dev/null; then
collect_error "Java is not installed or not in PATH"
return 1
fi
local version_str
version_str=$(java -version 2>&1 | head -n 1)
local current_version
if [[ "$version_str" =~ \"1\.([0-9]+) ]]; then
# Handle Java 8 format (e.g., "1.8.0_341")
current_version="${BASH_REMATCH[1]}"
else
# Handle Java 9+ format (e.g., "11.0.15")
current_version=$(echo "$version_str" | awk -F '"' '{print $2}' | awk -F '.' '{print $1}')
fi
info "Current Java version: $current_version (Required: ${required_version})"
if [[ "$current_version" != "$required_version" ]]; then
collect_error "Java version mismatch! Current: Java $current_version, Required: Java ${required_version}"
collect_error "Please switch to Java ${required_version} before running this script"
return 1
fi
success "Java version check passed: Java $current_version"
mark_check_passed
return 0
}| if [[ ! -d "${DIST_DIR}" ]]; then | ||
| collect_error "Directory ${DIST_DIR} does not exist" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
The script's argument parsing can be confusing if a user tries to specify a Java version without a local path. For example, ./validate-release.sh 1.7.0 user 11 would fail with a 'Directory 11 does not exist' error. While the README documents the "" workaround, improving the error message would greatly enhance usability and prevent user confusion by guiding them to the correct syntax.
| if [[ ! -d "${DIST_DIR}" ]]; then | |
| collect_error "Directory ${DIST_DIR} does not exist" | |
| exit 1 | |
| fi | |
| if [[ ! -d "${DIST_DIR}" ]]; then | |
| collect_error "Directory '${DIST_DIR}' does not exist." | |
| if [[ "${DIST_DIR}" =~ ^[0-9]+$ ]]; then | |
| collect_error "It looks like you may have tried to specify a Java version as the 3rd argument. To do so, provide an empty string for the local path, e.g.:\n ${SCRIPT_NAME} ${RELEASE_VERSION} ${USER} \"\" ${DIST_DIR}" | |
| fi | |
| exit 1 | |
| fi |
| find_package_dir() { | ||
| local pattern=$1 | ||
| local base_dir=${2:-"${DIST_DIR}"} | ||
|
|
||
| local found=$(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null | head -n1) | ||
|
|
||
| if [[ -z "$found" ]]; then | ||
| collect_error "Could not find directory matching pattern: $pattern" | ||
| return 1 | ||
| fi | ||
|
|
||
| echo "$found" | ||
| } |
There was a problem hiding this comment.
The find_package_dir function uses head -n1 to select a directory if multiple directories match the pattern. This could lead to unpredictable behavior if the file system contains unexpected directories. It's more robust to treat multiple matches as an error and report it to the user, ensuring the script always operates on the correct directory.
| find_package_dir() { | |
| local pattern=$1 | |
| local base_dir=${2:-"${DIST_DIR}"} | |
| local found=$(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null | head -n1) | |
| if [[ -z "$found" ]]; then | |
| collect_error "Could not find directory matching pattern: $pattern" | |
| return 1 | |
| fi | |
| echo "$found" | |
| } | |
| find_package_dir() { | |
| local pattern=$1 | |
| local base_dir=${2:-"${DIST_DIR}"} | |
| local -a found_dirs | |
| readarray -t found_dirs < <(find "$base_dir" -maxdepth 3 -type d -path "$pattern" 2>/dev/null) | |
| if [[ ${#found_dirs[@]} -eq 0 ]]; then | |
| collect_error "Could not find directory matching pattern: $pattern" | |
| return 1 | |
| elif [[ ${#found_dirs[@]} -gt 1 ]]; then | |
| collect_error "Found multiple directories matching pattern '$pattern':\n$(printf ' %s\n' "${found_dirs[@]}")" | |
| return 1 | |
| fi | |
| echo "${found_dirs[0]}" | |
| } |
2 participants
as title
Summary by CodeRabbit
文档更新
流程改进
其他