-
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
Fix IField.required
to not be required by default.
#105
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.
LGTM.
IField.required
to not be required by default.
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.
This seems like potentially a large semantic change (not everything pays attention to defaults), suggesting to me that the next release should be 6.1.0.
Could there be a test added to check not IField['required'].required
? Otherwise this could easily be changed back.
double checked in Plone content type schema editor - works now as expected |
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.
LGTM
Anyway, self referencing implementations are hard. In interfaces.py line 410 required=False
is redundant, but anyway, for the sake of being explicit (and documentation) I think this is fine.
This breaks the schema in eea.facettednavigation in a way that I do not understand:
The schema in question looks like this: class IFacetedSubtyper(Interface):
""" Support for subtyping objects
"""
can_enable = schema.Bool(u'Can enable faceted navigation',
readonly=True)
can_disable = schema.Bool(u'Can disable faceted navigation',
readonly=True)
is_faceted = schema.Bool(u'Is current object faceted navigable',
readonly=True)
is_lingua_faceted = schema.Bool(
u'Is LinguaPlone installed and current object is faceted navigable',
readonly=True) |
@agitator: Thanks for fixing my problem. |
Is there still something to be done before this PR can be merged? |
no other problems found so far, should be fine |
Just released as https://pypi.org/project/zope.schema/6.1.0/ to PyPI. |
Fixes #104