Added a git hook #7605
Added a git hook #7605
Conversation
Signed-off-by: ClementBouvierN <[email protected]>
📝 WalkthroughWalkthroughThis pull request introduces a new Bash script located in Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
bin/hook/pre-commit (1)
1-14: Add error handling and improve documentationWhile the basic structure is good, consider enhancing the script's robustness and documentation:
- Add command dependency checks
- Include usage instructions
- Document the expected file structure requirements
Add these improvements at the start of the script:
#!/bin/bash +# Verify required commands +for cmd in date grep sed git; do + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "Error: Required command '$cmd' not found" + exit 1 + fi +done + +# Usage instructions +if [ "$1" = "-h" ] || [ "$1" = "--help" ]; then + echo "Usage: pre-commit" + echo "Updates copyright years in modified files before commit." + echo "Supported files: .java, .ts, .html, .scss, .css, .adoc" + echo "Files must contain a copyright header in the format:" + echo "Copyright (c) YYYY, RTE (..." + exit 0 +fi + # Copyright (c) 2024, RTE (http://www.rte-france.com)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
bin/hook/pre-commit(1 hunks)src/docs/asciidoc/dev_env/scripts.adoc(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/docs/asciidoc/dev_env/scripts.adoc
| current_year=$(date +"%Y") | ||
|
|
||
| year_regex="([0-9]{4})(-[0-9]{4})?" | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve variable safety and regex precision
The variables should be protected against word splitting and globbing. Also, the regex could be more precise.
Apply these improvements:
-current_year=$(date +"%Y")
+current_year="$(date +"%Y")"
-year_regex="([0-9]{4})(-[0-9]{4})?"
+year_regex="(20[0-9]{2})(-20[0-9]{2})?" # More precise: matches years 2000-2099📝 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.
| current_year=$(date +"%Y") | |
| year_regex="([0-9]{4})(-[0-9]{4})?" | |
| current_year="$(date +"%Y")" | |
| year_regex="(20[0-9]{2})(-20[0-9]{2})?" # More precise: matches years 2000-2099 | |
|
|
||
| year_regex="([0-9]{4})(-[0-9]{4})?" | ||
|
|
||
| modified_files=$(git diff --cached --name-only --diff-filter=ACMR | grep -E '\.java$|\.ts$|\.html$|\.scss$|\.css$|\.adoc$') |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve file handling and make extensions configurable
The current file selection logic needs improvements for robustness and maintainability.
Apply these improvements:
-modified_files=$(git diff --cached --name-only --diff-filter=ACMR | grep -E '\.java$|\.ts$|\.html$|\.scss$|\.css$|\.adoc$')
+# Define monitored extensions
+readonly MONITORED_EXTENSIONS=( "java" "ts" "html" "scss" "css" "adoc" )
+
+# Build the grep pattern
+extension_pattern=$(printf "\\%s$|" ".${MONITORED_EXTENSIONS[@]}")
+extension_pattern="${extension_pattern%|}"
+
+# Get modified files (handles spaces in filenames)
+readarray -t modified_files < <(git diff --cached --name-only --diff-filter=ACMR | grep -E "$extension_pattern" || true)📝 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.
| modified_files=$(git diff --cached --name-only --diff-filter=ACMR | grep -E '\.java$|\.ts$|\.html$|\.scss$|\.css$|\.adoc$') | |
| # Define monitored extensions | |
| readonly MONITORED_EXTENSIONS=( "java" "ts" "html" "scss" "css" "adoc" ) | |
| # Build the grep pattern | |
| extension_pattern=$(printf "\\%s$|" ".${MONITORED_EXTENSIONS[@]}") | |
| extension_pattern="${extension_pattern%|}" | |
| # Get modified files (handles spaces in filenames) | |
| readarray -t modified_files < <(git diff --cached --name-only --diff-filter=ACMR | grep -E "$extension_pattern" || true) |
| # This file needs to be placed in operatorfabric-core/git/hooks/pre-commit | ||
| # This file has no extension and should stay as plain text |
There was a problem hiding this comment.
💡 Codebase verification
Pre-commit hook requires proper installation and permissions
The verification revealed several issues that need to be addressed:
- The pre-commit hook is currently located at
bin/hook/pre-commitbut needs to be moved to.git/hooks/pre-commit - The file permissions are
-rw-r--r--but the hook needs to be executable (+x) - There are no documented installation instructions in the README files
Required actions:
- Move the hook to
.git/hooks/pre-commit - Make the hook executable:
chmod +x .git/hooks/pre-commit - Add installation instructions to the repository documentation
🔗 Analysis chain
Verify hook installation process
The installation process for this git hook needs to be documented and verified:
- The hook needs to be executable
- The path needs to be corrected
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the hook installation instructions
# Check if there's a script to install the hook
if fd -t f "install.*hooks.*" || fd -t f "setup.*hooks.*"; then
echo "Found potential hook installation script"
fi
# Check if there's documentation about hook installation
rg -i "git.*hooks.*install" || rg -i "pre-commit.*hook.*install"
Length of output: 185
Script:
#!/bin/bash
# Let's verify the current state of the hook and related files
# Check the actual location and content of the pre-commit hook
echo "=== Checking pre-commit hook location ==="
fd -t f "pre-commit" .git/hooks/
fd -t f "pre-commit" bin/
# Check if there are any installation or setup instructions in README files
echo -e "\n=== Checking README files for hook setup instructions ==="
fd -t f "README" -E "*.md" -X cat {}
# Check file permissions of the pre-commit hook
echo -e "\n=== Checking pre-commit hook permissions ==="
fd -t f "pre-commit" -X ls -l {}
Length of output: 11151
| for file in $modified_files; do | ||
| # Check if the file contains the copyright header | ||
| if grep -q "Copyright (c)" "$file"; then | ||
| # Check if the header has a single year equal to the current year (e.g., 2024) | ||
| if grep -qE "Copyright \(c\) $current_year, RTE \(" "$file"; then | ||
| continue | ||
| fi | ||
|
|
||
| # Check if the header already has a year range (e.g., 2018-2023) | ||
| if grep -qE "Copyright \(c\) $year_regex, RTE \(" "$file"; then | ||
| # Replace the second year in the range with the current year (e.g., 2018-2023 becomes 2018-2024) | ||
| sed -i -E "s/Copyright \(c\) ([0-9]{4})(-[0-9]{4})?, RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file" | ||
| else | ||
| # If there's no year range, convert the single year to a range (e.g., 2023 becomes 2023-2024) | ||
| sed -i -E "s/Copyright \(c\) ([0-9]{4}), RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file" | ||
| fi | ||
| fi | ||
|
|
||
| # Add the file to the staging area after the modification | ||
| git add "$file" | ||
| done |
There was a problem hiding this comment.
Add error handling and improve file processing safety
The main processing loop needs several improvements for better reliability and safety:
- Handle sed failures
- Safe file iteration
- Backup files before modification
- Validate copyright format
Apply these improvements:
-for file in $modified_files; do
+for file in "${modified_files[@]}"; do
+ # Skip if file doesn't exist or isn't readable
+ if [ ! -r "$file" ]; then
+ echo "Warning: Cannot read file: $file"
+ continue
+ fi
+
+ # Create backup
+ cp "$file" "$file.bak"
+
# Check if the file contains the copyright header
if grep -q "Copyright (c)" "$file"; then
+ # Validate copyright format
+ if ! grep -qE "Copyright \(c\) ($year_regex), RTE \(" "$file"; then
+ echo "Warning: Invalid copyright format in $file"
+ continue
+ fi
+
# Check if the header has a single year equal to the current year
- if grep -qE "Copyright \(c\) $current_year, RTE \(" "$file"; then
+ if grep -qE "Copyright \(c\) ${current_year}, RTE \(" "$file"; then
+ rm -f "$file.bak" # Clean up backup
continue
fi
# Check if the header already has a year range
- if grep -qE "Copyright \(c\) $year_regex, RTE \(" "$file"; then
+ if grep -qE "Copyright \(c\) ${year_regex}, RTE \(" "$file"; then
# Replace the second year in the range with the current year
- sed -i -E "s/Copyright \(c\) ([0-9]{4})(-[0-9]{4})?, RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file"
+ if ! sed -i.tmp -E "s/Copyright \(c\) ([0-9]{4})(-[0-9]{4})?, RTE \(/Copyright \(c\) \1-${current_year}, RTE \(/" "$file"; then
+ echo "Error: Failed to update copyright in $file"
+ mv "$file.bak" "$file" # Restore backup
+ continue
+ fi
else
# If there's no year range, convert the single year to a range
- sed -i -E "s/Copyright \(c\) ([0-9]{4}), RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file"
+ if ! sed -i.tmp -E "s/Copyright \(c\) ([0-9]{4}), RTE \(/Copyright \(c\) \1-${current_year}, RTE \(/" "$file"; then
+ echo "Error: Failed to update copyright in $file"
+ mv "$file.bak" "$file" # Restore backup
+ continue
+ fi
fi
+ rm -f "$file.tmp" # Clean up sed temporary file
fi
+ # Clean up backup
+ rm -f "$file.bak"
+
# Add the file to the staging area after the modification
- git add "$file"
+ if ! git add "$file"; then
+ echo "Error: Failed to stage file: $file"
+ fi
done📝 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.
| for file in $modified_files; do | |
| # Check if the file contains the copyright header | |
| if grep -q "Copyright (c)" "$file"; then | |
| # Check if the header has a single year equal to the current year (e.g., 2024) | |
| if grep -qE "Copyright \(c\) $current_year, RTE \(" "$file"; then | |
| continue | |
| fi | |
| # Check if the header already has a year range (e.g., 2018-2023) | |
| if grep -qE "Copyright \(c\) $year_regex, RTE \(" "$file"; then | |
| # Replace the second year in the range with the current year (e.g., 2018-2023 becomes 2018-2024) | |
| sed -i -E "s/Copyright \(c\) ([0-9]{4})(-[0-9]{4})?, RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file" | |
| else | |
| # If there's no year range, convert the single year to a range (e.g., 2023 becomes 2023-2024) | |
| sed -i -E "s/Copyright \(c\) ([0-9]{4}), RTE \(/Copyright \(c\) \1-$current_year, RTE \(/" "$file" | |
| fi | |
| fi | |
| # Add the file to the staging area after the modification | |
| git add "$file" | |
| done | |
| for file in "${modified_files[@]}"; do | |
| # Skip if file doesn't exist or isn't readable | |
| if [ ! -r "$file" ]; then | |
| echo "Warning: Cannot read file: $file" | |
| continue | |
| fi | |
| # Create backup | |
| cp "$file" "$file.bak" | |
| # Check if the file contains the copyright header | |
| if grep -q "Copyright (c)" "$file"; then | |
| # Validate copyright format | |
| if ! grep -qE "Copyright \(c\) ($year_regex), RTE \(" "$file"; then | |
| echo "Warning: Invalid copyright format in $file" | |
| continue | |
| fi | |
| # Check if the header has a single year equal to the current year | |
| if grep -qE "Copyright \(c\) ${current_year}, RTE \(" "$file"; then | |
| rm -f "$file.bak" # Clean up backup | |
| continue | |
| fi | |
| # Check if the header already has a year range | |
| if grep -qE "Copyright \(c\) ${year_regex}, RTE \(" "$file"; then | |
| # Replace the second year in the range with the current year | |
| if ! sed -i.tmp -E "s/Copyright \(c\) ([0-9]{4})(-[0-9]{4})?, RTE \(/Copyright \(c\) \1-${current_year}, RTE \(/" "$file"; then | |
| echo "Error: Failed to update copyright in $file" | |
| mv "$file.bak" "$file" # Restore backup | |
| continue | |
| fi | |
| else | |
| # If there's no year range, convert the single year to a range | |
| if ! sed -i.tmp -E "s/Copyright \(c\) ([0-9]{4}), RTE \(/Copyright \(c\) \1-${current_year}, RTE \(/" "$file"; then | |
| echo "Error: Failed to update copyright in $file" | |
| mv "$file.bak" "$file" # Restore backup | |
| continue | |
| fi | |
| fi | |
| rm -f "$file.tmp" # Clean up sed temporary file | |
| fi | |
| # Clean up backup | |
| rm -f "$file.bak" | |
| # Add the file to the staging area after the modification | |
| if ! git add "$file"; then | |
| echo "Error: Failed to stage file: $file" | |
| fi | |
| done |
|
Nothing in release notes