Skip to content

Conversation

@PPsyrius
Copy link
Collaborator

Proposed change

  • Add Thai (th) l10n for Denmark holidays.
  • Add start_year as 1771 (based on "Helligdagsreformen af 1770"), also update the various observance starting point for OPTIONAL holidays accordingly.
  • Refactor test cases to modern library-wide standards.

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)
  • New feature (new holidays functionality in general)

Checklist

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

Summary by CodeRabbit

  • New Features

    • Added Thai language support for Danish holiday translations.
  • Improvements

    • Enhanced holiday accuracy for Denmark with historical year thresholds—Workers' Day and Constitution Day now reflect when they were actually observed, starting from 1890 and 1891 respectively.

Walkthrough

Adds Thai localization for Danish holidays, introduces start_year = 1771 in Denmark, includes th in supported languages, makes Workers' Day and Grundlovsdag optional based on year thresholds, adds Thai PO file, and refactors Denmark tests from year-specific to range-based suites.

Changes

Cohort / File(s) Summary
Docs
README.md
Denmark supported languages updated to include th.
Country implementation
holidays/countries/denmark.py
Added start_year = 1771; extended supported_languages to include Thai; made Workers' Day (added if year ≥ 1890) and Grundlovsdag/Constitution Day (added if year ≥ 1891) conditional/optional.
Localization
holidays/locale/th/LC_MESSAGES/DK.po
New Thai localization file mapping Danish holiday msgids to Thai msgstr translations.
Tests
tests/countries/test_denmark.py
Replaced year-specific tests with range-based tests (1771–2050); added many per-holiday tests and Thai localization checks; introduced class-level setup and an optional_holidays instance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • arkid15r
  • KJhellico

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Update Denmark holidays: add th l10n support, refactor test cases" directly captures the main changes in this changeset. The title specifically mentions the two primary objectives: adding Thai localization support for Denmark holidays and refactoring test cases to modern standards. While the title doesn't explicitly mention the start_year addition or optional holiday refinements, these are implementation details appropriately bundled under the umbrella of updating Denmark holidays. The title is concise, clear, and specific enough that a teammate scanning the history would immediately understand the primary work involved.
Description Check ✅ Passed The PR description is well-aligned with the actual changes in the changeset. It explicitly describes adding Thai localization for Denmark holidays, setting start_year to 1771 with rationale, updating optional holiday observance thresholds, and refactoring tests to modern standards. Each point mentioned corresponds to observable changes in the modified files. The description provides meaningful context and isn't vague or off-topic, making it a solid narrative of what's being changed and why.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • 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 a1ccfe3 and ad43c5e.

📒 Files selected for processing (1)
  • README.md (1 hunks)
🔇 Additional comments (1)
README.md (1)

560-561: Documentation update aligns with Thai localization addition.

The README has been correctly updated to reflect the addition of Thai (th) to Denmark's supported languages and the inclusion of the OPTIONAL category. The format is consistent with other country entries in the table.


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.

@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (04c2dae) to head (ad43c5e).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff            @@
##               dev     #3011   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          306       306           
  Lines        18069     18072    +3     
  Branches      2331      2333    +2     
=========================================
+ Hits         18069     18072    +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud
Copy link

Copy link
Collaborator

@KJhellico KJhellico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@arkid15r arkid15r added this pull request to the merge queue Oct 19, 2025
Merged via the queue into vacanza:dev with commit 785ce0d Oct 19, 2025
36 checks passed
@PPsyrius PPsyrius deleted the dk_th_l10n branch October 20, 2025 05:13
@KJhellico KJhellico mentioned this pull request Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants