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

Add syntactic sugar for getting the current Fiscal{Year,Quarter}. #4

Merged
merged 4 commits into from
Oct 22, 2019

Conversation

pmav99
Copy link
Contributor

@pmav99 pmav99 commented Feb 8, 2018

Fixes #2

I had to add pytest-mock in the dependencies.

I also thought that it would be better if a native speaker wrote the docs.

Anyway, check it out and let me know of any remarks.

@codecov-io
Copy link

codecov-io commented Feb 8, 2018

Codecov Report

Merging #4 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #4   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         254    262    +8     
=====================================
+ Hits          254    262    +8
Impacted Files Coverage Δ
fiscalyear.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3bf1e0...7655cae. Read the comment docs.

@adamjstewart
Copy link
Owner

Thanks! I'll try to get a chance to review this and your other PR sometime in the next week. I have exams coming up (student life...) so I apologize if I get side-tracked. Ping me if I don't respond in a week.

@pmav99
Copy link
Contributor Author

pmav99 commented Feb 16, 2018

ping!

Copy link
Owner

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty solid! Just a few minor comments.

fiscalyear.py Outdated Show resolved Hide resolved
fiscalyear.py Outdated Show resolved Hide resolved
Copy link
Owner

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple last minute changes.

README.rst Outdated Show resolved Hide resolved
docs/testing.rst Show resolved Hide resolved
@adamjstewart
Copy link
Owner

Ping @pmav99. Aside from my last 2 requested changes, I think this PR is ready to go!


.. code-block:: python

>>> from fiscalyear import *
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove import here too

@@ -26,13 +26,18 @@ The ``FiscalYear`` class provides an object for storing information about the st

.. code-block:: python

>>> from fiscalyear import *
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add this import back

@adamjstewart adamjstewart merged commit 58190b6 into adamjstewart:master Oct 22, 2019
@adamjstewart
Copy link
Owner

Thanks for the PR! I'll play around with the docs and do some testing and try to put out a new release by the end of the week. Sorry it took so long to get this merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Syntactic sugar for getting the current Fiscal{Year,Quarter}
3 participants