-
Notifications
You must be signed in to change notification settings - Fork 478
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 working day related calculations #2010
Conversation
- Update `HolidayBase::get_workdays_number()` logic - Update documentation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #2010 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 188 189 +1
Lines 11409 11456 +47
Branches 1791 1793 +2
=========================================
+ Hits 11409 11456 +47 ☔ View full report in Codecov by Sentry. |
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.
LGTM
@KJhellico @PPsyrius do you guys think we should update the v0
with this sort of a breaking change or just leave it for v1
?
Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com>
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.
Some naming options to consider (based on my chat with an AI bot):
HolidayBase::is_workday
toHolidayBase::is_working_day
HolidayBase::get_workdays_number
toHolidayBase::get_working_days_count
Quality Gate passedIssues Measures |
Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <ark@cho.red>
Proposed change
Update working day-related calculations:
HolidayBase::get_workdays_number()
logicType of change
Checklist
make pre-commit
, it didn't generate any changesmake test
, all tests passed locally