-
Notifications
You must be signed in to change notification settings - Fork 824
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
Allow better subclassing of MoneyField #11150
Allow better subclassing of MoneyField #11150
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.
Seems sensible. Thanks for contributing!
Couple of small changes.
If there's an existing test for buildCurrencyField()
, please add one for the new method as well.
your pr description
Please fill in all sections of the template, including linking to an issue.
If no issue currently exists, please create one. We track issues, not pull requests.
Please also tick all of the check boxes that apply - if you find that you can't tick a checkbox it probably means there's some more work for you to do (e.g. the commit message doesn't match the guidelines right now)
9b65a38
to
e237349
Compare
There was no test for this method, I am not even sure what you wan't to test here. Nevertheless, I've added two test methods. I will create a issue for this. |
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.
Thank you for the tests, that's fantastic! Exactly what I was after.
Just a couple of small things to tidy up, then I'll be happy to merge this.
Move generation of NumberField from constructor to method to allow override in subclass. Addded test for MoneyField
e237349
to
a3ce922
Compare
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.
Fantastic, thank you for implementing this and for your patience.
Description
Move generation of NumberField from constructor to method to allow override in subclass.
Manual testing steps
Issues
Pull request checklist