fix: add prominent commit format section to copilot instructions#408
Merged
Jamie-BitFlight merged 2 commits intomainfrom Nov 5, 2025
Merged
Conversation
- Add integration test for issue #335 to validate graceful handling of missing action.yml - Fix .git/config setup in test to prevent "No owner or repo found" error - Remove redundant await in test vi.mock() calls to fix ESLint errors - Add clarifying comments explaining __dirname usage for action.yml path resolution - Rebuild dist files Fixes #335
Add critical warning about conventional commits at the top of copilot-instructions.md with clear examples and type definitions to prevent future commit format mistakes.
|
Caution Review failedThe pull request is closed. WalkthroughThis PR updates documentation with commit message and Node version guidelines, removes unnecessary awaits from mock imports across test files, adds a new test for loading the action's own defaults, updates integration test expectations for error handling, and modifies production code to load defaults from the tool's own action.yml instead of a user's action.yml. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
Jamie-BitFlight
deleted the
claude/rebase-pr-400-011CUp8U5eFJ4FESLYU5ASDP
branch
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #335 by fixing a path resolution error when the tool is run via npx or yarn dlx. The main problem was that collectAllDefaultValuesFromAction() attempted to load the tool's own action.yml file, which doesn't exist when installed via npm (it's not in the "files" array). The fix gracefully handles this scenario by catching the error and continuing without default values.
Key Changes
- Added explanatory comments in
src/inputs.tsto clarify thatcollectAllDefaultValuesFromAction()loads the tool's own action.yml, not the user's - Updated integration test to validate the fix works when action.yml is missing
- Removed unnecessary
awaitkeywords from dynamic imports in test files (code style improvement) - Added commit message format guidelines to
.github/copilot-instructions.md
Reviewed Changes
Copilot reviewed 7 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/inputs.ts | Added clarifying comments explaining that the function loads the tool's own action.yml |
| tests/integration-issue-335.test.ts | Updated test to verify graceful handling of missing action.yml and simulate git repository |
| tests/inputs.test.ts | Added test case for loading tool's own action.yml and updated comments |
| tests/*.test.ts | Removed unnecessary await from dynamic imports |
| .github/copilot-instructions.md | Added commit message format guidelines |
| dist/* | Build artifacts regenerated from source changes |
| const gitDir = path.join(tempDir, '.git'); | ||
| fs.mkdirSync(gitDir); | ||
| const gitConfig = `[remote "origin"] | ||
| \turl = https://github.com/test-owner/test-repo.git |
There was a problem hiding this comment.
[nitpick] The git config uses a literal tab character (\\t) in the template string. While this works, it's more maintainable to use consistent whitespace. Consider using spaces or explicitly documenting why the tab is required (if the git config parser requires it).
Suggested change
| \turl = https://github.com/test-owner/test-repo.git | |
| url = https://github.com/test-owner/test-repo.git |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Type of Change
Related Issues
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes