Pull Request File Review

Pull Request File Review

Overview

Review pull request files to identify unnecessary test artifacts, temporary files, and non-production code based on naming patterns, locations, and usage.

When to Use

• Reviewing pull requests before approval
• Files with test-related names (testresults.md, test_output.csv)
• Temporary scripts or data files in the changeset
• Before merging to ensure only production-relevant files are included

Process

1. Identify Suspicious Files

Look for files matching these patterns:

• test*.md, *results.md, *output.md - Test result documentation
• test_*.csv, *_test_data.csv - Test data files
• scratch.py, temp.py, debug.py - Temporary scripts
• output.txt, results.json, debug.log - Output/log files
• Files in root directory that don't match project conventions

2. Evaluate File Necessity

For each suspicious file, ask:

Location Analysis:

• Is it in a test directory? (If yes, likely legitimate)
• Is it in a documentation directory with other docs? (If yes, likely legitimate)
• Is it in the project root or random location? (Red flag)

Usage Analysis:

• Is the file imported/referenced in production code?
• Is it required by tests that are committed?
• Is it part of CI/CD configuration?
• Does it have a clear purpose in the project structure?

Naming Convention:

• Does the name match the project's file naming conventions?
• Is it clearly temporary? (scratch, temp, test, output, debug)

3. Flag or Approve

Flag for removal if:

• File appears to be a one-off test artifact
• Name suggests temporary/debug purpose
• Not referenced anywhere in the codebase
• Located in an unusual place for its type

Approve if:

• File is referenced in production code
• Part of the test suite infrastructure
• Documented purpose in project structure
• Follows project conventions

4. Provide Feedback

When flagging files:

🚫 Unnecessary: `path/to/file.ext`
Reason: [Test artifact/temporary script/etc.]
Evidence: [Not referenced/unusual location/temporary naming]
Recommendation: Remove from PR or relocate

Examples

Example 1: Test Results File

Context: PR includes testresults.md in project root

Application:

1. Location: Root directory (suspicious)
2. Naming: "testresults" indicates test artifact
3. Search: grep -r "testresults.md" finds no references
4. Decision: Flag for removal

Outcome:

🚫 Unnecessary: `testresults.md`
Reason: Test artifact in root directory
Recommendation: Remove from PR

Example 2: Test Data Referenced in Tests

Context: PR includes tests/fixtures/sample_data.csv

Application:

1. Location: tests/fixtures/ (appropriate)
2. Naming: Follows fixture convention
3. Search: Referenced in tests/test_parser.py
4. Decision: Approve

Outcome: ✅ Necessary test fixture

Example 3: Temporary Debug Script

Context: PR includes debug_api.py in root

Application:

1. Location: Root directory (suspicious)
2. Naming: "debug" indicates temporary purpose
3. Search: No imports found
4. Decision: Flag for removal

Outcome:

🚫 Unnecessary: `debug_api.py`
Reason: Temporary debug script in root
Recommendation: Remove from PR or add to .gitignore if needed locally

Example 4: Configuration File

Context: PR includes test_config.yaml

Application:

1. Location: Root directory
2. Naming: Contains "test" but may be configuration
3. Search: Referenced in tests/conftest.py
4. Documentation: Mentioned in testing README
5. Decision: Approve

Outcome: ✅ Legitimate test configuration

Anti-patterns

• ❌ Don't: Flag every file with "test" in the name

  • ✅ Do: Analyze context, location, and usage

• ❌ Don't: Approve files just because they're small

  • ✅ Do: Apply consistent criteria regardless of file size

• ❌ Don't: Require deep code analysis for obvious artifacts

  • ✅ Do: Use file name and location as primary signals

• ❌ Don't: Flag files without checking references first

  • ✅ Do: Search the codebase for imports/references before flagging

Testing This Skill

To validate this skill:

1. Create test PR with mixed files:

   • testresults.md in root (should flag)
   • tests/fixtures/data.csv referenced in tests (should pass)
   • scratch.py in root (should flag)
   • Legitimate config with test-like name (should pass after analysis)

2. Apply skill systematically:

   • Follow the 4-step process for each file
   • Document reasoning for each decision

3. Verify accuracy:

   • All flagged files should be unnecessary
   • No legitimate files should be flagged
   • Feedback should be clear and actionable

Quick Reference Commands

# List all files in PR
git diff --name-only main...HEAD

# Check if file is referenced in codebase
grep -r "filename.ext" --exclude-dir=.git

# Search for imports (Python example)
grep -r "from.*filename import\|import.*filename" .

# Find test-like files
find . -name "*test*" -o -name "*debug*" -o -name "*temp*" -o -name "*scratch*"

---

Remember: Catch obvious artifacts without creating friction. When uncertain, ask the author.