Skip to content

fix: code review improvements - docs, scripts, and tooling#1

Merged
mmcky merged 3 commits intomainfrom
fix/code-review-improvements
Feb 23, 2026
Merged

fix: code review improvements - docs, scripts, and tooling#1
mmcky merged 3 commits intomainfrom
fix/code-review-improvements

Conversation

@mmcky
Copy link
Contributor

@mmcky mmcky commented Feb 23, 2026

Code Review Improvements

Fixes identified during a detailed technical review of the entire repository.

Documentation Fixes

  • docs/testing.md — Rewrote from scratch. Was 1,275 lines of severely corrupted/interleaved duplicate content; now clean 222 lines with all testing scenarios, troubleshooting, and validation checklist.
  • tests/README.md — Rewrote from scratch. Was 357 lines of duplicated content referencing deleted test files (test-improvements.sh, test-show-report.sh, test-report-preview.sh); now clean 97 lines referencing only the three test files that actually exist.
  • CONTRIBUTING.md — Updated from a generic Python project template (referenced pip install -r requirements-dev.txt, pytest tests/, PEP 8) to accurate shell-project instructions with correct tooling (bash, curl, jq) and actual test commands.
  • docs/hyperlinks.md — Fixed overview text that said "metrics greater than 1" when the actual code behavior is "metrics greater than 0".

Script Fixes

  • generate-report.sh — Removed 8 no-op shift statements inside for arg in "$@" loop (shift has no effect in a for loop; harmless but misleading).
  • generate-report.sh — Changed misleading ERROR: GITHUB_OUTPUT environment variable not set! to a debug-level message in CLI mode, since GITHUB_OUTPUT is only relevant when running as a GitHub Action.

Tooling & Process

  • .github/copilot-instructions.md — Compressed from 411 to 119 lines. Removed generic sections (code style, best practices, anti-patterns, common tasks, development workflow) and kept project-specific rules: no summary files, .tmp/ file staging, gh CLI temp file patterns, release process. Fixed a duplicate post-release verification block.
  • .gitignore — Added .tmp/ working directory for file staging.
  • CHANGELOG.md — Updated [Unreleased] section with all fixes.

- Rewrite corrupted docs/testing.md (1275 -> 222 lines, deduplicated)
- Rewrite corrupted tests/README.md (357 -> 97 lines, references only existing files)
- Update CONTRIBUTING.md from Python template to shell-project instructions
- Remove no-op shift statements in generate-report.sh arg parsing
- Fix misleading GITHUB_OUTPUT error message in CLI mode
- Fix docs/hyperlinks.md stating '> 1' when behavior is '> 0'
- Add .tmp/ file staging guidelines to copilot-instructions.md
- Add .tmp/ to .gitignore
- Update CHANGELOG.md with all fixes
Remove generic sections (code style, best practices, anti-patterns,
common tasks, development workflow, testing) that any AI already knows.
Keep project-specific rules: no summary files, .tmp/ staging, gh CLI
temp files, release process. Fix duplicate post-release verification block.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR cleans up and corrects repository documentation and small script/tooling details for the weekly report GitHub Action, addressing issues found during a repo-wide review.

Changes:

  • Rewrites/cleans corrupted or outdated documentation (docs/testing.md, tests/README.md, CONTRIBUTING.md) and corrects hyperlink behavior docs.
  • Simplifies generate-report.sh CLI arg parsing by removing no-op shifts and downgrades a misleading CLI-mode GITHUB_OUTPUT message to debug.
  • Updates tooling/process files: .gitignore adds .tmp/, Copilot instructions are condensed, and CHANGELOG.md records the fixes.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/README.md Rewritten test documentation to match existing scripts and current CI behavior.
generate-report.sh Removes no-op shifts in for arg in "$@" parsing; adjusts CLI-mode GITHUB_OUTPUT messaging.
docs/testing.md Rewritten testing guide with clearer scenarios, CI notes, and troubleshooting.
docs/hyperlinks.md Corrects hyperlink rule to “metrics > 0”.
CONTRIBUTING.md Updates contribution/dev setup instructions from Python template to shell/action reality.
CHANGELOG.md Adds [Unreleased] entries describing the doc/script/tooling fixes.
.gitignore Adds .tmp/ staging directory.
.github/copilot-instructions.md Condenses instructions and adds .tmp/ staging guidance + gh temp-file patterns.

- Relax Bash 4.0+ requirement to just Bash (script works with macOS default 3.2)
- Remove exact line counts from CHANGELOG entries to avoid drift
@mmcky mmcky merged commit 396f0e1 into main Feb 23, 2026
3 checks passed
@mmcky mmcky deleted the fix/code-review-improvements branch February 23, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants