Documented the overridden form options #5313
Conversation
javiereguiluz
commented
May 25, 2015
javiereguiluz
commented
| Q | A |
|---|---|
| Doc fix? | no |
| New docs? | yes |
| Applies to | all |
| Fixed tickets | #3412 |
|
On the first question: Yes, you don't have to use includes. It's just to avoid duplication. If there still is a lot of copy/past, you can decide to use placeholders like in For the side note, it already has an issue: #5213 |
javiereguiluz
changed the title
|
This PR is now ready to be reviewed. Thanks. |
|
|
||
| **type**: ``boolean`` **default**: ``false`` | ||
|
|
||
| This option specifies if the type is compound. This is independent of whether |
There was a problem hiding this comment.
this doesn't explain what "compound" means.
There was a problem hiding this comment.
You are right! I'm waiting for the general fix of this issue: #5213
| @@ -30,6 +30,8 @@ day and year) or three select boxes (see the `widget`_ option). | |||
| | | - `years`_ | | |||
| +----------------------+-----------------------------------------------------------------------------+ | |||
| | Overridden Options | - `by_reference`_ | | |||
There was a problem hiding this comment.
options imho should be lowercased (to be consistent with "Inherited options").
|
I've updated the description of the |
| | | - `compound`_ | | ||
| | | - `data_class`_ | | ||
| | | - `error_bubbling`_ | | ||
| +----------------------+-----------------------------------------------------------------------------+ |
There was a problem hiding this comment.
This block should be moved below the "Options" block.
There was a problem hiding this comment.
and the same applies to the actual descriptions below
|
@javiereguiluz I left you some last minor comments. After that, this looks ready to be merged. |
|
@xabbuh didn't we remove types from the overriden options? (as it's more about documenting the new value and why than actually documenting the option) |
|
@wouterj At least, we are not consistent with this rule then: http://symfony.com/doc/current/reference/forms/types/entity.html#choice-list, http://symfony.com/doc/current/reference/forms/types/choice.html#compound By the way, does it really make sense to leave out the type? I would then always have to look the type up in the parent type. That's not really comfortable, is it? |
|
@xabbuh I've fixed all the issues that you found and I've rebased the PR. I guess this is now ready to be merged. Thanks! |
| | Overriden | - `choice_list`_ | | ||
| | options | - `choices`_ | | ||
| | Overridden | - `choices`_ | | ||
| | options | - `choice_list`_ | |
There was a problem hiding this comment.
This should be reverted (see #5343).
|
👍 Looks great now! |
This PR was merged into the 2.3 branch. Discussion ---------- Documented the overridden form options | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes | Applies to | all | Fixed tickets | #3412 Commits ------- 84633db Fixed a typo 596a0bc Reordered two overridden options 62a11d5 Added missing types in overridden options 2b63f24 Fixed the order of the different options e14b650 Fixed the order of the different options 0a5781b Improved the explanation about the "compoun" option eb20dc8 Fixer minor issues 974dfef Fixed errors noticed by Wouter 32c0af1 Removed a duplicated "trim" option in the "password" type 1dc53fa Removed again a duplicated option explanation 7f8e09a Removed a duplicated option explanation in "file" type 48b402b Extracted the common "data_class" option explanation for data-related types f44e971 Documented the overridden options of "date" type e41b3cc Documented overridden options for "file" type 8e825d9 Documented the overridden options of the "password" type 618e11d Documented the overridden options for "time" type 7ce8191 Documented overridden options for hidden field a8ad338 Documented overriden options for numeric types e66ec5c Created a compound_type file because this option is shared with lots of types 8d360d9 Documented the Overridden Options of the Text type
|
Wow, BIG work - that's incredible. Thanks Javier! So now, as long as code changes show up as docs issues/PR's, we can stay in sync permanently. Minor reordering tweaks at sha: f28ef78. There is some inconsistency with the order of the sections, but I think that's ok - sometimes overridden seems more important than inherited and vice-versa. And really, in a perfect world, I'm not sure if we need these different sections at all - does a user care of it's inherited? And I might also want the order of the options to be non-alphabetical, and instead to be based on how common an option is. Just thinking aloud :) Cheers! |
