-
Notifications
You must be signed in to change notification settings - Fork 22
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
Changed period into abstract class and moved docstrings #25
base: master
Are you sure you want to change the base?
Conversation
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.
Can you do me a favor, and replace all occurences of 'element' by 'entry'? It's a remainder from the tinydb implementation.
Also, can you check why the travis build failed? You can install the dev tools locally, and run code and style tests yourself (described in 'Contributions' in the README).
financeager/period.py
Outdated
:raise: PeriodException if validation failed or table name unknown | ||
:return: ID of new entry (int) | ||
""" | ||
pass |
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 can remove all the pass
. Having a docstring alone already is a syntactically valid way to define a class.
Already passed the pre-commit test. Not sure why the build failed |
Did you have a look at the build log? You can do so by clicking 'Details'. |
I'm not sure how to test an abstract class. Should I delete this function? |
Ah that's tricky. Have a look at this (I found it by googling for 'python test abstract class method'). This means you could do
|
Can you rebase this onto master? |
Can you also move the methods |
Do you mean cut and copy these methods into Period class? Should I keep the staticmethod decorators? |
I noticed that the change is still a bit trickier... I'd like to have the methods of |
No description provided.