Add Afghanistan holidays #2198
Add Afghanistan holidays #2198
Conversation
There was a problem hiding this comment.
@Prateekshit73, it's a good start! But I have some issues about the holidays.
…add-afghanistan Merge from main branch to sync the fork.
…lidays into add-afghanistan Merge from main branch to sync the fork.
|
I tried my best, but I apologize for the inconvenience caused. I think that rather than helping, I am making it worse. |
|
@Prateekshit73 , you don't need to apologize, it's a work process. We will discuss everything and eventually achieve a result. ✌️ |
@Prateekshit73 you're doing great. Don't take the review process as a critique of your work. It's a standard way to achieve a greater goal of having accurate holidays calendar for |
There was a problem hiding this comment.
Glad to see new contributors 😊
Let me know once you've nailed down all date assignments debate with @KJhellico so that I can help out with the test suites.
|
@Prateekshit73 Hi - I've created a code review suggestion PR on your forked branch which includes the following changes:
|
Afghanistan Code Review Suggestions
…lidays into add-afghanistan syncing the fork with the main repository.
…lidays into add-afghanistan syncing the fork with the main repository.
|
There was a problem hiding this comment.
@Prateekshit73 thank you for your hard work on this clean PR!
A new country support is one of the best contribution in terms of impact.
LGTM!
@KJhellico @PPsyrius I've changed fa to fa_AF -- please review one more time.
Proposed change
Added Afghanistan holidays, based on Public holidays in Afghanistan on wikipedia, Today's and Upcoming Holidays in Afghanistan
Type of change
holidaysfunctionality in general)Checklist
make check, all checks and tests are green