feat!: remove default .docsignore value from configuration

[cawpea]

Issue

#11

BREAKING CHANGE: The ignoreFile configuration no longer defaults to '.docsignore'. To continue using .docsignore, explicitly set ignoreFile: '.docsignore' in your configuration file (.coderefrc.json or package.json).

This change makes the ignore file behavior explicit rather than implicit, applying ignore patterns only when intentionally configured. All documentation examples now use .gitignore instead of .docsignore to reflect this generic approach.

🤖 Generated with Claude Code

[github-actions]

PR Review: Remove default .docsignore value from configuration

Summary

This PR makes a breaking change to remove the default .docsignore value from the ignoreFile configuration option, requiring users to explicitly specify an ignore file if they want to use one. The implementation is clean and the change is well-documented.

✅ Strengths

1. Clear Breaking Change Communication: The PR description and documentation updates clearly communicate the breaking change and migration path.

2. Consistent Implementation: The change is applied consistently across:

   • Configuration system (src/config.ts:82)
   • Tests (src/config.test.ts, src/core/validate.test.ts, src/utils/fix.test.ts)
   • Documentation (README.md, docs/user-guide/configuration.md)
   • CLI output messages (src/cli/validate.ts)

3. Test Coverage: All tests have been updated to reflect the new behavior, including:

   • Default configuration tests now expect undefined for ignoreFile
   • Explicit configuration tests properly set the ignoreFile value

4. Documentation Quality: The configuration documentation includes a clear migration note explaining the version change and how to maintain previous behavior.

🔍 Issues & Recommendations

1. CHANGELOG Missing (Medium Priority)

The CHANGELOG.md file exists but hasn't been updated with this breaking change. This is important for users upgrading.

Recommendation: Add an entry to CHANGELOG.md:

## [Unreleased]

### Changed

- **BREAKING**: Removed default `.docsignore` value from `ignoreFile` configuration. Users must now explicitly set `ignoreFile` in their configuration if they want to use ignore patterns from a file. ([#11](https://github.com/cawpea/coderef/issues/11))

### Migration Guide

If you were relying on the default `.docsignore` file, add this to your `.coderefrc.json`:
\`\`\`json
{
  "ignoreFile": ".docsignore"
}
\`\`\`

2. Function Name Inconsistency (Low Priority)

The function loadDocsignorePatterns in src/utils/ignore-pattern.ts:56 still references "docsignore" in its name, even though it's now a generic ignore file loader.

Recommendation: Consider renaming to loadIgnorePatterns for consistency:

export function loadIgnorePatterns(ignoreFilePath: string): string[] {
  // ... implementation
}

This would better reflect the generic nature of the ignore file support.

3. JSDoc Comments in Japanese (Low Priority)

The file src/utils/ignore-pattern.ts contains JSDoc comments in Japanese (lines 5, 8, 13, 29, 34, 47, 54, 65). While not a bug, this is inconsistent with the rest of the codebase which uses English comments.

Recommendation: Translate comments to English for consistency:

/**
 * Check if a file path matches a pattern (simple glob matching)
 */
export function matchesPattern(filePath: string, pattern: string): boolean {
  // Directory pattern (ends with /)
  // ... etc
}

4. CLI Output Messages Updated Correctly (Positive Note)

Good attention to detail in updating the CLI output messages in src/cli/validate.ts:106-122 to use generic "ignore patterns" terminology instead of specific ".docsignore" references.

🔒 Security Assessment

No security concerns identified. The change removes a default behavior but doesn't introduce any vulnerabilities.

⚡ Performance Assessment

No performance impact. The change is purely configuration-related.

🧪 Test Coverage

Test coverage appears adequate:

• Configuration loading tests updated
• Integration tests in CLI updated
• Mock configurations in validation tests updated

Note: All tests were updated to remove the ignoreFile: '.docsignore' line, which is correct for the new default behavior.

📚 Documentation Assessment

Well Documented:

• README.md updated with .gitignore example
• User guide updated with migration note
• Configuration documentation clearly states the change

Missing:

• CHANGELOG.md needs an entry (as noted above)
• Consider adding a migration guide or upgrade notes document if this is part of a larger 0.2.0 release

🎯 Overall Assessment

This is a well-executed breaking change with good attention to detail. The main issues are minor:

1. Missing CHANGELOG entry (should be added before merge)
2. Function naming inconsistency (nice-to-have cleanup)
3. Japanese comments (code quality improvement)

Recommendation: ✅ Approve with minor changes

• Add CHANGELOG entry before merging
• Consider the function renaming and comment translation as follow-up improvements

Code Style Compliance

✅ Follows coding standards from CLAUDE.md:

• 2-space indentation maintained
• Single quotes used consistently
• Conventional commit format followed (feat!:)
• TypeScript best practices maintained

Great work on maintaining consistency throughout this breaking change!

[github-actions]

PR Review: Remove default .docsignore value from configuration

Summary

This PR implements a breaking change to remove the default .docsignore value from the configuration system. The change makes ignore file behavior explicit rather than implicit, which is a sensible design decision that improves clarity and flexibility.

---

✅ Code Quality & Best Practices

Strengths:

• Clean, focused changes that follow the single responsibility principle
• Proper use of TypeScript optional types (ignoreFile?: string)
• Consistent code style matching the project standards (2-space indentation, single quotes, semicolons)
• Good separation of concerns - changes are isolated to config and documentation

Observations:

• The implementation correctly removes the default value from getDefaultConfig() (src/config.ts:79-84)
• The getIgnoreFilePath() function properly handles the undefined case by returning null (src/config.ts:282-287)
• All affected code paths properly check for null/undefined before using the ignore file path

---

✅ Potential Bugs or Issues

No major issues found. The changes are well-implemented:

• Null safety is properly handled in getIgnoreFilePath() and loadDocsignorePatterns() call sites
• The fallback to empty array ([]) in src/cli/validate.ts:108 is appropriate
• Tests have been updated to reflect the new behavior (checking for undefined instead of .docsignore)

---

✅ Performance Concerns

No performance concerns. The changes maintain the same performance characteristics:

• The ignore file is still loaded only when configured
• No additional I/O operations introduced
• Pattern matching logic remains unchanged

---

✅ Security Concerns

No security concerns. The changes actually improve security posture slightly:

• Removing implicit defaults reduces unexpected behavior
• Users must explicitly opt-in to ignore file behavior
• No new attack vectors introduced

---

⚠️ Test Coverage

Adequate but could be enhanced:

The PR updates existing tests in src/config.test.ts:

• Line 42: Removed assertion for default .docsignore value ✅
• Line 221: Updated to expect undefined instead of .docsignore ✅
• Lines 322-328: Modified test to explicitly set ignoreFile when testing the path ✅
• Lines 351: Updated to expect null instead of path ✅

Recommendations for additional test coverage:

1. Add a test case verifying that ignore patterns are correctly skipped when ignoreFile is undefined
2. Consider adding an integration test showing the CLI behavior when no ignore file is configured
3. Test the environment variable override (CODEREF_IGNORE_FILE) to ensure it works when set

The tests removed from src/core/validate.test.ts and src/utils/fix.test.ts are appropriate since these tests just needed mock configs updated.

---

⚠️ Documentation Update Needs

Good documentation updates, but migration guidance could be clearer:

What's well documented:

• README.md updated to show .gitignore example ✅
• Configuration guide (docs/user-guide/configuration.md) updated with:
  • New default value documented as undefined ✅
  • Migration note added (lines 67-68) ✅
  • All examples updated to use .gitignore ✅
  • Section heading changed from ".docsignore Syntax" to "Ignore File Syntax" ✅

Recommendations:

1. Add a CHANGELOG entry or migration guide - Users upgrading from previous versions need to know:

   • This is a BREAKING CHANGE (already mentioned in PR body ✅)
   • How to maintain existing behavior (set ignoreFile: '.docsignore' explicitly)
   • Consider adding this to a CHANGELOG.md or docs/migration/v0.2.0.md

2. Consider updating the CLI help text - If there's CLI help output, ensure it reflects that ignore files are now optional

3. README.md breaking change notice - Consider adding a note in the README about this breaking change for v0.2.0 (similar to the note in configuration.md)

---

📋 Commit Message Compliance

Excellent commit message formatting:

• Follows Conventional Commits specification ✅
• Uses feat!: prefix correctly indicating breaking change ✅
• Includes detailed BREAKING CHANGE: footer explaining the impact ✅
• According to docs/development/git-conventions.md, feat! should trigger MAJOR version bump
• Co-authored with Claude Code properly attributed ✅

---

🎯 Overall Assessment

This is a high-quality PR that is ready to merge with minor recommendations.

Score: 9/10

Strengths:

• Clean implementation with proper null safety
• Well-structured breaking change with clear migration path
• Comprehensive documentation updates
• Proper test updates
• Good commit message hygiene

Minor improvements suggested:

1. Add a CHANGELOG.md entry or migration guide document
2. Consider additional test coverage for the undefined ignore file case
3. Verify CLI help text reflects optional nature of ignore files

Recommendation: ✅ APPROVE - This PR is well-implemented and follows project conventions. The suggested improvements are minor and non-blocking.

---

Great work on making the configuration more explicit and flexible! The shift from .docsignore to allowing any ignore file (including .gitignore) is a positive change that will reduce confusion and improve the developer experience.

[github-actions]

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 0.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀