fix: use org-repo cspell config for spell check in check-pr-title#83
Conversation
The spell check step was running in the project-repo working directory, so cspell would look for cspell.config.yml in the calling repo (which often doesn't have one). This caused false positives where words already defined in the org-repo's cspell.config.yml (like 'setuptools', 'pyproject') were flagged as unknown. Now pass --config pointing to the org-repo's cspell.config.yml, matching the pattern already used by the committed step with committed.toml. Fixes #76
WalkthroughThe ChangesSpell check configuration fallback
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Hopefully this change will work |
|
It should work, but I don't want this.
Keeping the list of misspellings in the org repo
The pre-commit workflow is the only reusable workflow that cpp-linter-rs is still using. If you merge this, then I'll have to switch to a project-specific copy. |
|
BTW, I don't really care if a dependency name causes a spelling error in the PR title. I've been merging those dependabot PRs without adding the dep name to the cspell config's list. I added the "check PR title" so we could prevent misspellings in the release notes and change logs. But I also added the change logs to cspell's list of ignored files. Only the cpp-linter-rs keeps proper change logs because it is deploying 4 packages from 1 repo. |
I see. I was just do not introduce cspell.config.yml to the project, so want to maitain it here. If so, I am good to leave it for now. |
|
That's a lot of effort to avoid creating 1 file. If you want, we can add an input option to skip the spell-check in the pre-commit workflow. |
|
Spell-check in the pre-commit is useful. If some input or condition can point to the cspell.config.yml here(org-level), that would be great. (by default it sitll point to project level cspell.config.yml) |
How about - name: spell check
env:
PR_TITLE: "${{ steps.get-title.outputs.title }}"
- CSPELL_CONFIG: ${{ github.workspace }}/org-repo/cspell.config.yml
- run: echo "${PR_TITLE}" | npx cspell-cli lint --config "${CSPELL_CONFIG}" stdin
+ run: |
+ if [ -f project-repo/cspell.config.yml ]; then
+ CSPELL_CONFIG="project-repo/cspell.config.yml"
+ else
+ CSPELL_CONFIG="org-repo/cspell.config.yml"
+ fi
+ echo "${PR_TITLE}" | npx cspell-cli lint --config "${CSPELL_CONFIG}" stdin |
|
Ah, that would be more agreeable, yes. |
If project-repo/cspell.config.yml exists, use it for spell check. Otherwise, fall back to org-repo/cspell.config.yml. This addresses the concern that each project should be able to maintain its own cspell dictionary rather than accumulating all words in the org-level config.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/pre-commit.yml (1)
88-93: 💤 Low valueConsider defensive validation for missing org config.
The conditional correctly prefers project config and falls back to org config. If the org config is also missing, cspell-cli will fail with a config-not-found error. While unlikely (the org repo is under maintainer control), adding an explicit check could provide a clearer failure message.
🛡️ Optional defensive check
run: | if [ -f project-repo/cspell.config.yml ]; then CSPELL_CONFIG="project-repo/cspell.config.yml" else CSPELL_CONFIG="org-repo/cspell.config.yml" fi + if [ ! -f "${CSPELL_CONFIG}" ]; then + echo "Error: cspell config not found at ${CSPELL_CONFIG}" + exit 1 + fi echo "${PR_TITLE}" | npx cspell-cli lint --config "${CSPELL_CONFIG}" stdin🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pre-commit.yml around lines 88 - 93, The current if/else that sets CSPELL_CONFIG prefers project-repo/cspell.config.yml then org-repo/cspell.config.yml but does not handle the case where neither exists; update the block that sets CSPELL_CONFIG to check both locations and, if both are missing, emit a clear error message (e.g. via echo) and exit non‑zero so the workflow fails with a readable message; ensure the variable CSPELL_CONFIG is only used after this validation so cspell-cli won’t receive a missing-config path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/pre-commit.yml:
- Around line 88-93: The current if/else that sets CSPELL_CONFIG prefers
project-repo/cspell.config.yml then org-repo/cspell.config.yml but does not
handle the case where neither exists; update the block that sets CSPELL_CONFIG
to check both locations and, if both are missing, emit a clear error message
(e.g. via echo) and exit non‑zero so the workflow fails with a readable message;
ensure the variable CSPELL_CONFIG is only used after this validation so
cspell-cli won’t receive a missing-config path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02de58bf-0ac9-442a-97f9-f136c08bce45
📒 Files selected for processing (1)
.github/workflows/pre-commit.yml
2bndy5
left a comment
2bndy5
left a comment
There was a problem hiding this comment.
I'm glad we could reach a compromise here. 🎉 for teamwork!
2 participants
Problem
The
check-pr-titlejob's spell check step was running withworking-directory: project-repo, socspell-clilooked forcspell.config.ymlin the calling repo (e.g., cpp-linter-hooks). Most repos in the org don't have their own cspell config, causing false positives where words already defined in the org-repo'scspell.config.yml(likesetuptools,pyproject) were flagged as unknown.Fixes #76.
Solution
Pass
--configpointing to the org-repo'scspell.config.yml, matching the pattern already used by thecommittedstep (which uses--config "${COMMITTED_CONFIG}").Before
After
CSpell supports
--configflagCSpell CLI has a
-c, --configoption to specify a configuration file to use, so no need to replace it with another tool.Summary by CodeRabbit