|
|
DescriptionPatch Set 1 #
Total comments: 4
Patch Set 2 : Do not encourage to overwrite #Patch Set 3 : Remove trailing spaces #
Total comments: 1
Patch Set 4 : Use reference for attributes #
Total comments: 2
Patch Set 5 : Use attr instead of reference #
Total comments: 1
Patch Set 6 : Use ~ to not render de context #
Total comments: 2
MessagesTotal messages: 18
https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst#new... doc/ref/fields.rst:72: trailing spaces should be removed https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst#new... doc/ref/fields.rst:74: idem https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst#new... doc/ref/fields.rst:75: ``cls.<field name>.readonly = <PYSON expresion>`` It is not possible to use PYSON into readonly attributes. It should be true or false. So you should set it into false first and then sett the states with the pyson expression.
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/447091003
Sign in to reply to this message.
The improvement should be only on explaining the precedence. https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/423231004/doc/ref/fields.rst#new... doc/ref/fields.rst:69: have precedence over states. So instead of doing: I do not want to encourage people to do that. It is a bad practice.
Sign in to reply to this message.
Do not encourage to overwrite
Sign in to reply to this message.
Remove trailing spaces
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/447091003
Sign in to reply to this message.
https://codereview.tryton.org/447091003/diff/421371003/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/421371003/doc/ref/fields.rst#new... doc/ref/fields.rst:68: The ``required`` or ``readonly`` attributes have precedence over Better to use reference to required and readonly attributes.
Sign in to reply to this message.
Use reference for attributes
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/447091003
Sign in to reply to this message.
https://codereview.tryton.org/447091003/diff/421401003/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/421401003/doc/ref/fields.rst#new... doc/ref/fields.rst:72: The `required <_field_readonly>` or `readonly <_field_readonly>` Why not use attr?
Sign in to reply to this message.
Use attr instead of reference
Sign in to reply to this message.
https://codereview.tryton.org/447091003/diff/421401003/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/421401003/doc/ref/fields.rst#new... doc/ref/fields.rst:72: The `required <_field_readonly>` or `readonly <_field_readonly>` On 2022/06/21 21:38:00, ced wrote: > Why not use attr? Because I'm not familiar with sphinx or how it's build the Tryton docs, on previous comment you mention the reference and that's what I look for. Now I undestand the attr, still not fully as I don't see the diference between :attr:`Field.required` and :attr:`~Field.required`.
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/447091003
Sign in to reply to this message.
https://codereview.tryton.org/447091003/diff/421411003/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/421411003/doc/ref/fields.rst#new... doc/ref/fields.rst:68: The :attr:`Field.required` or :attr:`Field.readonly` attributes have You can use ~ so only the last part is rendered as we do not need the context as we are in the same one.
Sign in to reply to this message.
Use ~ to not render de context
Sign in to reply to this message.
checks OK URL: https://codereview.tryton.org/447091003
Sign in to reply to this message.
https://codereview.tryton.org/447091003/diff/425531005/doc/ref/fields.rst File doc/ref/fields.rst (right): https://codereview.tryton.org/447091003/diff/425531005/doc/ref/fields.rst#new... doc/ref/fields.rst:68: The :attr:`~Field.required` or :attr:`~Field.readonly` attributes have I would say 'and' instead of 'or'. https://codereview.tryton.org/447091003/diff/425531005/doc/ref/fields.rst#new... doc/ref/fields.rst:68: The :attr:`~Field.required` or :attr:`~Field.readonly` attributes have I would also say that it is only if they are `True`.
Sign in to reply to this message.
|