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
Add Ireland #152
Add Ireland #152
Conversation
* Merge ireland.py * Adding Ireland * Add Ireland tests * Add Ireland * Add Ireland * Ireland tests * Ireland changelog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to add a few test to cover the conditional holidays (1974, 1977, 1994...) and fix the pep8 errors detected by flake8
@@ -4,7 +4,7 @@ CHANGELOG | |||
master (unreleased) | |||
------------------- | |||
|
|||
- Nothing changed yet. | |||
- Add Ireland. thx @gregn610 () |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may want to add the reference to your pull request (#152)
"Saint Patrick substitute")) | ||
|
||
# Whit Monday | ||
if year <= 1973: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you'd need to add a test to check that, before 1974, this day was added to the holidays
)) | ||
|
||
# May Day | ||
if year >= 1994: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, that be great to test that before 1994, this wasn't a holiday
days.append(self.get_june_holiday(year)) | ||
days.append(self.get_august_holiday(year)) | ||
|
||
if year >= 1977: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idem ; check before and after 1977.
self.assertIn(date(2015, 12, 28), holidays) # St. Stephen's day shift | ||
|
||
|
||
class IrelandTest(GenericCalendarTest): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably a copy-paste error, you don't need this duplicate instance of IrelandTest ;o)
Don't give up! your contribution is close to be accepted, just a few fixes and it's going to be fine. |
Ireland code review fixes
Excellent! welcome to workalendar, Ireland! |
For information, read and make sure you're okay with the CONTRIBUTING document.
<country>
by@pseudo
(#)"