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.

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

Update Juneteenth with observed dates #282

Merged
merged 2 commits into from
Dec 28, 2022

Conversation

tylershuster
Copy link
Contributor

No description provided.

@stelgenhof
Copy link
Member

Hi @tylershuster Thanks for the PR! Can you please add some unit tests? Please see also: https://github.com/azuyalabs/yasumi/blob/develop/CONTRIBUTING.md
Thanks!

@tylershuster
Copy link
Contributor Author

Will do. Sorry about the commit history; I'll clean that up. I'm a little confused about the existing unit tests because it seems to already test for the change I added! (minus the "(observed)" portion)

@tylershuster
Copy link
Contributor Author

Please pardon my tardiness. Things seem to be in order, so please review.

@stelgenhof
Copy link
Member

@tylershuster Reran the checks and there seem to be some code style issues.

@stelgenhof
Copy link
Member

@tylershuster Can you check once more your PR as there some minor errors? I'd like to include this fix in the next release I am planning. Thanks!

@tylershuster
Copy link
Contributor Author

@stelgenhof I think the style is fine now — PHPCS doesn't throw any errors.

@stelgenhof stelgenhof self-requested a review September 7, 2022 01:59
@stelgenhof stelgenhof added this to the 2.6 milestone Sep 7, 2022
src/Yasumi/Provider/USA.php Outdated Show resolved Hide resolved
src/Yasumi/Provider/USA.php Outdated Show resolved Hide resolved
src/Yasumi/Provider/USA.php Outdated Show resolved Hide resolved
@tylershuster
Copy link
Contributor Author

Thanks for the feedback. I think those issues are cleaned up. I tried to pattern it after an existing substitute holiday

tests/USA/JuneteenthTest.php Outdated Show resolved Hide resolved
@tylershuster
Copy link
Contributor Author

Failure seems...incorrect

@stelgenhof
Copy link
Member

The unsuccessful checks are not directly related to this PR. I will merge it and resolve these afterwards.

@stelgenhof stelgenhof merged commit a81952d into azuyalabs:develop Dec 28, 2022
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.

None yet

2 participants