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

ListField: removed default None value for the items_types parameter. #97

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

avrahamshukron
Copy link
Contributor

This will create more consistent code since item_types is required.
Tests had to be modified a bit. Specifically tests which used Model
class with circular reference to itself. It was easier to just extract
the dummy class outside the function, in order to avoid import path hell.

This change breaks the current API, so if merged it should be into a new
major version.

Fixes #96

This will create more consistent code since `item_types` _is_ required.
Tests had to be modified a bit. Specifically tests which used Model
class with circular reference to itself. It was easier to just extract
the dummy class outside the function, in order to avoid import path hell.

This change breaks the current API, so if merged it should be into a new
major version.
@avrahamshukron avrahamshukron changed the title ListField: removed default None value for the items_type parameter. ListField: removed default None value for the items_types parameter. Oct 9, 2017
@beregond
Copy link
Collaborator

beregond commented Aug 7, 2018

@avrahamshukron nice, and you are right - therefore lets make version 3 :) before that I need to release some small improvements to branch 2.x and then we will do all BC breaking stuff (this and regexes too)

@avrahamshukron
Copy link
Contributor Author

Sure, thanks!
Please hold the release of 3.0 until the weekend, I might have some more small fixes to push.

@beregond
Copy link
Collaborator

Ok, no problem

@avrahamshukron
Copy link
Contributor Author

I totally forgot about this... If it is still relevant you can absolutely merge this.

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

Successfully merging this pull request may close these issues.

2 participants