-
Notifications
You must be signed in to change notification settings - Fork 14
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 FiscalDay #13
Add FiscalDay #13
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 1 1
Lines 364 485 +121
==========================================
+ Hits 364 485 +121
Continue to review full report at Codecov.
|
Since there is overlap between this and your FiscalMonth PR, I'm going to focus on reviewing the other PR first. |
Merged master, so should be good to review now. |
Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>
Added some simple documentation. Does it make sense to add sections for FiscalMonth and FiscalDay to the readme? Not sure why the thought didn't occur to me with the last PR, but even if it's a bit redundant it might help someone identify that the library is relevant for their use case. Also, I added very subtle documentation for the |
Yes, please add to the README.rst and basics.rst, either in this PR or a follow-up PR.
I think that's good. It will also appear in the Sphinx-generated API docs. |
Hey @adamjstewart here's a possible implementation. Let me know what you think. I can add documentation if/when you feel good about the PR.