Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Palau holidays #1776

Merged
merged 5 commits into from
Apr 26, 2024
Merged

Add Palau holidays #1776

merged 5 commits into from
Apr 26, 2024

Conversation

PPsyrius
Copy link
Collaborator

Proposed change

Add Palau holidays (ARMED_FORCES, HALF_DAY, PUBLIC).

Closes #1238.

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/upgrade)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (a code change causing existing functionality to break)
  • New feature (new python-holidays functionality in general)

Checklist

  • I've followed the contributing guidelines
  • I've run make pre-commit, it didn't generate any changes
  • I've run make test, all tests passed locally

@PPsyrius
Copy link
Collaborator Author

PPsyrius commented Apr 24, 2024 •
edited
Loading

Version ~3.8.0-0 was not found in the local cache
Error: The version '3.8' with architecture 'arm64' was not found for macOS 14.4.1.
The list of all available versions can be found here: https://raw.githubusercontent.com/actions/python-versions/main/versions-manifest.json

The CI/CD appears to be broken. @KJhellico @arkid15r

@KJhellico
Copy link
Collaborator

The CI/CD appears to be broken.

Yes, I saw this error in my repository earlier today.

@arkid15r arkid15r mentioned this pull request Apr 24, 2024
Closed
@arkid15r
Copy link
Collaborator

The CI/CD appears to be broken. @KJhellico @arkid15r

The issue was resolved in #1778

@coveralls
Copy link

coveralls commented Apr 24, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 8825640354

Details

  • 49 of 49 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 8820337570: 0.0%
Covered Lines: 11052
Relevant Lines: 11052
💛 - Coveralls

KJhellico
KJhellico reviewed Apr 24, 2024
View reviewed changes
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.

Well done! 👍 Just a few suggestions

holidays/countries/palau.py Outdated Show resolved Hide resolved
holidays/countries/palau.py Outdated Show resolved Hide resolved
holidays/countries/palau.py Outdated Show resolved Hide resolved
holidays/countries/palau.py Outdated Show resolved Hide resolved
tests/countries/test_palau.py Outdated Show resolved Hide resolved
tests/countries/test_palau.py Outdated Show resolved Hide resolved
tests/countries/test_palau.py Outdated Show resolved Hide resolved
tests/countries/test_palau.py Outdated Show resolved Hide resolved
@PPsyrius PPsyrius requested a review from KJhellico April 25, 2024 03:51
KJhellico
KJhellico reviewed Apr 25, 2024
View reviewed changes
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!

arkid15r
arkid15r reviewed Apr 26, 2024
View reviewed changes
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.

Looks good, great contribution!

A couple of comments to address:

holidays/countries/palau.py Outdated Show resolved Hide resolved
tests/countries/test_palau.py Outdated Show resolved Hide resolved
@PPsyrius @arkid15r
[suggestion] 2024Apr26 suggestions
7f58be9 
Co-authored-by: Arkadii Yakovets <[email protected]>
@PPsyrius PPsyrius requested a review from arkid15r April 26, 2024 15:54
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@arkid15r arkid15r enabled auto-merge April 26, 2024 16:01
arkid15r
arkid15r approved these changes Apr 26, 2024
View reviewed changes
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.

🇵🇼 LGTM

@arkid15r arkid15r added this pull request to the merge queue Apr 26, 2024
Merged via the queue into vacanza:dev with commit 48f85eb Apr 26, 2024
29 checks passed
@PPsyrius PPsyrius deleted the palau_holidays branch April 30, 2024 03:46
@KJhellico KJhellico mentioned this pull request May 6, 2024
Merged
@KJhellico KJhellico mentioned this pull request Oct 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KJhellico KJhellico KJhellico left review comments

@arkid15r arkid15r arkid15r approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Palau holidays
4 participants
@PPsyrius @KJhellico @arkid15r @coveralls