python-holidays icon indicating copy to clipboard operation
python-holidays copied to clipboard

Added spell checking functionality to pre-commit hooks and Makefile.

Open Prateekshit73 opened this issue 3 months ago • 3 comments

Proposed change

Added spell checking functionality to pre-commit hooks and Makefile. Resolves #2308.

Type of change

  • [ ] New country/market holidays support (thank you!)
  • [ ] Supported country/market holidays update (calendar discrepancy fix, localization)
  • [ ] Existing code/documentation/test/process quality improvement (best practice, cleanup, refactoring, optimization)
  • [ ] Dependency update (version deprecation/pin/upgrade)
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [ ] Breaking change (a code change causing existing functionality to break)
  • [x] New feature (new holidays functionality in general)

Checklist

  • [x] I've read and followed the contributing guidelines.
  • [x] I've run make check locally; all checks and tests passed.

Prateekshit73 avatar Sep 06 '25 16:09 Prateekshit73

Summary by CodeRabbit

  • New Features

    • Added spell-checking to the default check workflow, covering code, docs, and localization files.
    • Cross-platform support via Docker-based and Windows scripts.
    • Automated sorting of a custom dictionary for consistent results.
  • Chores

    • Introduced a containerized spell-check toolchain and supporting configuration.
    • Expanded clean tasks to remove additional build and cache artifacts.
  • Documentation

    • Help/usage output now lists the spellcheck command.
  • Style

    • Minor formatting adjustments to pre-commit configuration (no behavioral changes).

Walkthrough

Adds cspell-based spellcheck infra: Docker image, npm manifest and dictionaries, cspell config and empty custom dictionary, Makefile and Windows wrappers to run spellcheck, a script to sort the custom dictionary, and a minor pre-commit formatting tweak. No production code changes.

Changes

Cohort / File(s) Summary
Pre-commit config formatting
\.pre-commit-config.yaml
Minor formatting/indentation and quote normalization only; no behavioral change.
Build orchestration (Unix + Windows)
Makefile
Makefile, make.cmd
Adds spellcheck target, .PHONY: spellcheck, sort-custom-dict target, integrates spellcheck into check, extends clean, updates help. Windows make.cmd adds :Sort-custom-dict and :Spellcheck, wires Call :Spellcheck into :Check.
cspell toolchain (config, Docker, wrappers, package, dict)
cspell/*
cspell/Dockerfile, cspell/Makefile, cspell/make.cmd, cspell/package.json, cspell/cspell.json, cspell/custom-dict.txt
New Dockerfile to run cspell, local cspell Makefile and Windows wrapper to build/run container, npm manifest with cspell + many dictionaries, cspell.json configuration, and a placeholder custom-dict.txt.
Utility script
tools/sort_cspell_dict.py
New script sort_custom_dict() that reads cspell/custom-dict.txt, deduplicates, sorts, and writes it back with a trailing newline.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The changes partially satisfy issue #2308: the PR selects cspell and adds cspell configuration, a Docker image, a package.json with many dictionaries, Makefile targets, Windows helpers, and a sorting tool for a custom dictionary, which addresses tooling selection and multi-language support locally; however it does not actually add a cspell pre-commit hook (only formatting edits to .pre-commit-config.yaml), provides no CI workflow integration, and the custom dictionary file is empty, so the primary linked objectives (pre-commit and CI integration and a populated custom dictionary) remain unmet. To satisfy the linked-issue requirements, add and commit a cspell entry to .pre-commit-config.yaml, add a CI job (e.g., GitHub Actions) that runs make spellcheck or the cspell Docker target, populate cspell/custom-dict.txt with curated project terms or generate it from l10n sources, and document IDE/CI setup and install steps; update the PR once those items are implemented.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title is concise and accurately highlights the main repository change—adding spell-checking and Makefile integration; however it also claims pre-commit hook changes while the .pre-commit-config.yaml in the diff only shows formatting-level edits and no added spell-check hook, so the title is only partially accurate to the full changeset.
Out of Scope Changes Check ✅ Passed All added or modified files are focused on introducing cspell-based spell checking and supporting tooling (Makefile, cspell/*, tools script, Windows helpers); there are no unrelated codebase or API changes detected beyond reasonable maintenance tweaks to Makefile clean targets.
Description Check ✅ Passed The PR description is on-topic and directly related to the changeset, stating that spell-checking was added and linking the issue it intends to resolve, so it meets this lenient check.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 336879a9f6a689c30c7a380e65d6c7d9c61b8fee and cc11806f6aec4bb5746cbdca773be94c85db2e13.

📒 Files selected for processing (1)
  • .pre-commit-config.yaml (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: PPsyrius
PR: vacanza/holidays#2608
File: holidays/locale/en_US/LC_MESSAGES/VC.po:76-76
Timestamp: 2025-06-10T05:08:07.939Z
Learning: For the holidays project, localization file formatting issues (like missing terminal periods in .po files) should be automatically fixed by running `make l10n` command (which is included in `make check`). Authors should be directed to use this automated tooling instead of manual formatting fixes.
🔇 Additional comments (2)
.pre-commit-config.yaml (2)

23-25: Formatting-only tweak to pyupgrade — OK.

No functional impact. Keep as-is.


64-64: Quote style change — OK; tox-ini-fmt rev is current (1.6.0).

Verified latest release is 1.6.0 (released July 30, 2025); no later releases or breaking changes since 1.6.0.

[!TIP]

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Sep 06 '25 16:09 coderabbitai[bot]

It still reports 25,729 unknown words. Should I add custom dictionaries like I did for Hindi, or is there another recommended workaround?

Prateekshit73 avatar Sep 07 '25 03:09 Prateekshit73