Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[2.8][Form] Deprecate alias tag option#15926
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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.
we need a deprecation to be triggered when the service id is used, because such case makes no sense actually.
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.
I agree that it doesn't mean anything, but it's the default at the moment. Do you think we should require setting theextended_type attribute and trigger an exception if it (or alias) is not set?
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.
@wouterj yes if neither the alias or extended_type is set, it should trigger a deprecation as well. in 3.0 it should then throw an exception.
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.
please move this to theelse block which makes it more clear what code can be removed later
d048496 to3a62981Comparewouterj commentedSep 30, 2015
PR updated |
Tobion commentedOct 1, 2015
Status: Needs Work |
3a62981 tof15bfddComparewouterj commentedOct 1, 2015
Unless I missed something, I've now finished this PR:
|
f15bfdd to9b76e71Comparewouterj commentedOct 1, 2015
Status: Needs Review |
wouterj commentedOct 1, 2015
Build failures are not related to this PR btw |
UPGRADE-2.8.md Outdated
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.
Maybe also useextended_type/extended-type esp. since the following is code is xml which will cause confusion.
wouterj commentedOct 1, 2015
Fixed, thanks@Tobion |
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 is useless
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.
fixed
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.
Well, it is useless, but it doesn't hurt that much and reduces the changes to be done in 3.0.
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.
in 3.0 we will throw an exception, there is no defaultnull
Tobion commentedOct 1, 2015
Thank you@wouterj. |
…ibutes (WouterJ)This PR was merged into the 3.0-dev branch.Discussion----------[WIP][3.0][Form] Removed usage of deprecated form tag attributes| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | yes| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -WIP because it needs some adjustments after#15926 is mergedCommits-------215fdbe Removed alias attribute usages
FQCN should be used since 2.8 instead, so a deprecation error should be triggered when the
aliassetting is used.Furthermore, the name of the option doesn't make much sense for form types (as it's the alias of the field it applies to), so I renamed it to
extended_type. I'm open to any other suggestions.