Skip to content

Conversation

@boonya
Copy link
Contributor

@boonya boonya commented Jan 23, 2016

Issue #16

@boonya boonya force-pushed the 16-mark-field-as-required branch from ed49c59 to 5c08c79 Compare January 23, 2016 01:23
@boonya boonya force-pushed the 16-mark-field-as-required branch from 5c08c79 to 7d2f3d9 Compare January 23, 2016 01:34
@boonya
Copy link
Contributor Author

boonya commented Jan 23, 2016

Dude, seriously, I have no idea what does this error mean https://travis-ci.org/ets-labs/domain_models/jobs/104227992#L121

@rmk135
Copy link
Member

rmk135 commented Jan 23, 2016

@boonya, great job! Thanks!

One thing here here that would be nice to add - add required=False argument for fields.Model and fields.Collection initializers and pass it to the parent initializer call (like it is done for default argument in these fields).

I didn't use **kwargs for the child fields that have overridden initializer and retained default arg obviously defined, because of making easier IDE introspection and autosuggestion when someone will define a field (**kwargs - is easier to maintain, but worse for the end-user).

@rmk135
Copy link
Member

rmk135 commented Jan 23, 2016

About the tests failing on 3.2 - it's a travis problem. I checked on our other project - situation is the same. So, no worries.

@boonya boonya force-pushed the 16-mark-field-as-required branch from 036ae58 to 05d96b3 Compare January 24, 2016 16:25
@boonya
Copy link
Contributor Author

boonya commented Jan 24, 2016

@rmk135, like this one? a662609

@boonya
Copy link
Contributor Author

boonya commented Jan 24, 2016

BTW, I have added Travis CI & Coveralls bages into README

@rmk135
Copy link
Member

rmk135 commented Jan 24, 2016

Yeah, like this! Thanks!

@rmk135
Copy link
Member

rmk135 commented Jan 24, 2016

Regarding the badges - looks good!

I think that will make it like there https://github.com/ets-labs/dependency_injector/blob/master/README.rst in future.

rmk135 added a commit that referenced this pull request Jan 24, 2016
#16: Possibility to mark filed as required.
@rmk135 rmk135 merged commit 51e4f8c into master Jan 24, 2016
@boonya
Copy link
Contributor Author

boonya commented Jan 24, 2016

Of course. In future it will be. I think we will add much more bages ;)

@boonya boonya deleted the 16-mark-field-as-required branch January 25, 2016 08:54
@boonya
Copy link
Contributor Author

boonya commented Jan 25, 2016

I've added more badges ;) 2e84f1e

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.

3 participants