Update Lebanon holidays: add French language support#3407
Conversation
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds French localization for Lebanon: README updated, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Hi, I’ve opened a PR adding French localisation for Lebanon holidays. Happy to make any adjustments if needed! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@holidays/countries/lebanon.py`:
- Line 46: supported_languages is indented with 3 spaces causing a syntax error;
align it with the surrounding class body by using 4-space indentation so the
tuple assignment (supported_languages = ("ar", "en_US", "fr")) is at the same
indentation level as other class attributes/methods (e.g., inside the Lebanon
class or equivalent) without changing the tuple contents.
In `@tests/countries/test_lebanon.py`:
- Around line 359-391: The test_l10n_fr expected strings in test_l10n_fr do not
match the updated French translations in LB.po; for dates with multiple holiday
names (e.g., 2025-04-18, 2025-04-21 and the other mismatched entries) update the
tuples in test_l10n_fr to use the exact French strings from
holidays/locale/fr/LC_MESSAGES/LB.po and ensure when multiple names share a date
they are alphabetically ordered within the string (separated by the same
delimiter used in LB.po); locate and edit the expected tuples in the
test_l10n_fr function to match LB.po exactly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce7390dd-316a-478a-be52-c323ac21df7c
📒 Files selected for processing (4)
README.mdholidays/countries/lebanon.pyholidays/locale/fr/LC_MESSAGES/LB.potests/countries/test_lebanon.py
PPsyrius
left a comment
There was a problem hiding this comment.
Minor fixes aside, feel free to add your name to the CONTRIBUTORS file as well - the rest are LGTM👍
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Refactored the test_l10n_fr method to include updated holiday names and added the method back after it was previously removed. Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/countries/test_lebanon.py`:
- Around line 327-328: Move the def test_l10n_default out of the nested scope so
it's a top-level test method in the test class (currently it's nested inside
test_2024); reduce its indentation to the class-level method indent (4 spaces)
and indent its body to the standard method body indent (8 spaces) so the method
signature is at the same indent as other test methods and unittest discovery
will find it; verify the method body lines are consistently indented and not
left-aligned with the def.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19a3211b-bcf2-4bca-811d-04dd905f39d5
📒 Files selected for processing (1)
tests/countries/test_lebanon.py
|
Thank you for the feedback! Ive sorted the minor issues and added my name to the contributors list. Id love to connect too |
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTORS`:
- Line 193: The CONTRIBUTORS file entry "Osama Alnajar" is out of alphabetical
order; remove the current line containing "Osama Alnajar" and insert it between
the lines for "Osayd Abdu" and "Oscar Romero" so the list remains sorted by
first name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4d891150-44be-4651-9d7e-345eff164ed3
📒 Files selected for processing (1)
CONTRIBUTORS
PPsyrius
left a comment
There was a problem hiding this comment.
Please look at the 2 remaining issues still left open for this PR as marked by the bot, others are LGTM 👍
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
…and included their links) Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Added additional references for Lebanon holidays. Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
Co-authored-by: Panpakorn Siripanich <19505219+PPsyrius@users.noreply.github.com> Signed-off-by: Osama Alnajar <osama.alnajar.26@gmail.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #3407 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 310 312 +2
Lines 18585 18643 +58
Branches 2380 2382 +2
=========================================
+ Hits 18585 18643 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thanks everyone, really appreciate the reviews! 🙌 |
Proposed change
This PR adds French localisation support for Lebanon holidays.
Changes:
Type of change
holidaysfunctionality in general)Checklist
make checklocally; all checks and tests passed.