MLE-30056: Add update copyright skill - #620
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new script has correctness/portability issues in exclusion matching, diff parsing, and tail usage that can lead to incorrect results or failures on BSD/macOS environments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new Copilot skill and supporting script to deterministically check/update Progress Software copyright headers in this repository, computing per-file end years from each file’s Git history.
Changes:
- Add skill prompt/instructions for running a deterministic copyright header checker/updater.
- Add a self-contained bash + git script to compute target years and apply header updates.
- Add human-facing documentation for the skill and script usage/exclusions.
File summaries
| File | Description |
|---|---|
| .github/skills/update-copyright-headers/SKILL.md | Skill definition and agent instructions/guardrails for running check/update flows. |
| .github/skills/update-copyright-headers/scripts/copyright_headers.sh | Core deterministic checker/updater implementation (bash + git). |
| .github/skills/update-copyright-headers/README.md | Human documentation for activation, exclusions, and direct script usage. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if [[ "$pattern" == *'*'* ]]; then | ||
| regex="^${pattern//\*/.*}\$" | ||
| if [[ "$relpath" =~ $regex ]]; then | ||
| return 0 | ||
| fi | ||
| fi |
| local diff_text="$1" line content | ||
| while IFS= read -r line || [[ -n "$line" ]]; do | ||
| case "$line" in | ||
| '+++'*|'---'*) continue ;; |
| if [[ -n "$(tail -c1 -- "$path" 2>/dev/null)" ]]; then | ||
| FILE_HAS_TRAILING_NEWLINE=0 | ||
| else | ||
| FILE_HAS_TRAILING_NEWLINE=1 | ||
| fi |
| (( failed > 0 )) && exit 1 | ||
| exit 0 | ||
| else | ||
| (( count_needs > 0 || count_missing > 0 )) && exit 1 |
There was a problem hiding this comment.
count_error doesn't appear to be considered when choosing exit status. Is that intentional or does it need to be added?
| abs="$resolved_dir/$base" | ||
| fi | ||
| relpath_from "$abs" "$repo_root" | ||
| RELPATHS+=("$RELPATH_RESULT") |
There was a problem hiding this comment.
Copilot's calling this one out to me, so it might be nitpicking, but the script doesn't seem to verify the relative path is in the repo. If the script is invoked on something like "/tmp/random-file.txt" outside the repo, the relative path could turn into "../../random-file.txt" and the script will still proceed to rewrite it. That might give an agent too much flexibility to invoke the script on out-of-scope files and be a bit of a risk it will modify things it shouldn't be touching.
| if (( i == n-1 )) && (( FILE_HAS_TRAILING_NEWLINE == 0 )); then | ||
| printf '%s' "${FILE_LINES[$i]}" | ||
| else | ||
| printf '%s\n' "${FILE_LINES[$i]}" |
There was a problem hiding this comment.
It looks like a missing header may create mixed line endings in a CRLF file. Existing lines retain their \r, but the newly generated header lines do not have one, and write_file_lines appends only \n. Since I don’t see a .gitattributes rule enforcing LF for Java files, should the script detect and preserve the file’s existing line-ending style?
Summary
update-copyright-headersCopilot skill for Java source files.