Added support for default values in Inclusive schemas#382
Added support for default values in Inclusive schemas#382alecthomas merged 3 commits intoalecthomas:masterfrom
Conversation
|
It's a trivial change but I think there should be a test case to ensure that this is automatically tested and continues to behave as expected. |
|
I did think about that. Right now there is no test case for |
|
@svisser I created a set of new tests for both |
|
It seems a useful change to make to me. Given that it's new functionality, I wonder if @alecthomas has an opinion on this? |
alecthomas
left a comment
alecthomas
left a comment
There was a problem hiding this comment.
LGTM with one minor change, thanks!
voluptuous/schema_builder.py
Outdated
|
|
||
| def __init__(self, schema, group_of_inclusion, msg=None, description=None): | ||
| def __init__(self, schema, group_of_inclusion, | ||
| msg=None, default=UNDEFINED, description=None): |
There was a problem hiding this comment.
Add new parameters at the end to avoid unnecessary breakage.
There was a problem hiding this comment.
OK. I put them in that order because that is the order used by both Required and Optional (which is its parent class) but I'm happy either way.
There was a problem hiding this comment.
I've pushed d6e83a6 to address this. Reordering the arguments is the only thing that commit does, so If you decide that consistency with the other classes is more compelling than consistency with the part then feel fee to ignore it.
There was a problem hiding this comment.
Ah understood, but still I think avoiding potential breakage is preferable.
4 participants
This is a trivial implementation of a fix for #381.