Skip to content

Conversation

@Ulyzzeus
Copy link

@Ulyzzeus Ulyzzeus commented Dec 27, 2025

Proposed change

Added school holidays for every subdivision of Germany.

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

Ulyzzeus and others added 8 commits December 26, 2025 20:17
Updated project version and revision date in localization file. Added German holiday terms for localization.

Signed-off-by: Ulyzzeus <103255220+Ulyzzeus@users.noreply.github.com>
Updated project version and revision date in localization file. Added new holiday translations for various terms.

Signed-off-by: Ulyzzeus <103255220+Ulyzzeus@users.noreply.github.com>
Signed-off-by: Ulyzzeus <103255220+Ulyzzeus@users.noreply.github.com>
Updated project version and revision date in DE.po. Added new holiday terms for localization.

Signed-off-by: Ulyzzeus <103255220+Ulyzzeus@users.noreply.github.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 27, 2025

Caution

Review failed

Failed to post review comments

Summary by CodeRabbit

Release Notes

  • New Features

    • Germany holiday calendar now includes school holidays as a new category alongside public and Catholic holidays, with year-specific schedules for all German states.
  • Localization

    • Translations expanded for German holiday terms, now supporting English, Thai, and Ukrainian languages for broader international accessibility.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

This PR adds SCHOOL holiday category support to Germany with per-year and per-subdivision population methods. Includes version bump to 0.88, localization updates across multiple languages, and comprehensive test coverage for school holiday ranges.

Changes

Cohort / File(s) Summary
Documentation
README.md
Updated supported categories to include SCHOOL alongside CATHOLIC and PUBLIC for Germany.
Core Implementation
holidays/countries/germany.py
Added SCHOOL to supported\categories, introduced self.expand = False in \\init\\_, implemented year-specific school holiday population methods for 15+ German subdivisions (HH, BB, BE, BW, BY, HE, MV, NI, NW, RP, SH, SL, SN, ST, TH), overridden \_populate to orchestrate subdivision-specific logic with cross-year cleanup.
Localization
holidays/locale/de/LC_MESSAGES/DE.po, holidays/locale/en_US/LC_MESSAGES/DE.po, holidays/locale/th/LC_MESSAGES/DE.po, holidays/locale/uk/LC_MESSAGES/DE.po
Bumped version metadata from 0.87 to 0.88 across all locales, added new msgid/msgstr pairs for German holiday terms with translations in English, Thai, and Ukrainian (German DE.po entries remain untranslated).
Test Coverage
tests/countries/test_germany.py
Added extensive subdivision-specific tests validating school holiday ranges across 2025–2030+, cross-year Christmas periods, Easter, Pfingsten, Sommerferien, per-year population verification, and localization checks.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested labels

l10n, test

Suggested reviewers

  • arkid15r
  • KJhellico
  • PPsyrius

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: adding school holidays support for every German subdivision.
Description check ✅ Passed The description is directly related to the changeset, stating the core purpose of adding school holidays for every German subdivision.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
6.4% Duplication on New Code (required ≤ 5%)

See analysis details on SonarQube Cloud

# (keeps per-year population semantics for school holiday checks).
self.expand = False

def _add_school_holidays_hh(self, year):
Copy link
Collaborator

Choose a reason for hiding this comment

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

To add school holidays on a per subdiv basis, you can use def _populate_subdiv_{subdiv code}_public_holidays(self): but I would recommend moving these after each subdiv-specific public holidays assignment

Suggested change
def _add_school_holidays_hh(self, year):
def _populate_subdiv_hh_school_holidays(self):

Comment on lines +129 to +137
def _add_holiday_range(name, start_date, end_date):
current_date = start_date
while current_date <= end_date:
if current_date.year == self._year:
# Ensure holiday is added for the populated year by passing
# month and day to _add_holiday so it constructs the date
# using `self._year` (prevents cross-year leakage).
self._add_holiday(name, current_date.month, current_date.day)
current_date += timedelta(days=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We actually have self._add_multiday_holiday method for this - i.e.

if self._year >= 2007:

        if self._year >= 2007:
            # New Harvest Days.
            self._add_multiday_holiday(self._add_holiday_sep_5(tr("Шинэ ургацын өдрүүд")), 45)

elif self._year <= 1925:

            elif self._year <= 1925:
                self._add_multiday_holiday(
                    # Songkran New Year Holidays.
                    self._add_holiday_mar_28(tr("พระราชพิธีตะรุษะสงกรานต์ แลนักขัตฤกษ์")),
                    18,
                )

@PPsyrius PPsyrius marked this pull request as draft December 27, 2025 03:42
@PPsyrius PPsyrius changed the title Added school holidays for every subdivision of Germany Update Germany holidays: add SCHOOL holidays category support Dec 27, 2025
holidays_map = []

if year == 2025:
holidays_map = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better to rely not just on a list of dates, but to identify the rules that they follow.

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