Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form] Fix BC break introduced in #14403#18275
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
HeahDude commentedMar 23, 2016
| Q | A |
|---|---|
| Branch? | 2.8 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | n/a |
| License | MIT |
| Doc PR | - |
HeahDude commentedMar 23, 2016
HeahDude commentedMar 23, 2016
Note to merger: to revert in 3.0 |
xabbuh commentedMar 23, 2016
Why is the order of the attributes important? Imo you should not rely on it. |
HeahDude commentedMar 23, 2016
@xabbuh we don't have the choice IMHO. The component themes need to print the value independently from {%setread_only=true %}{{block('parent_block') }} |
mnapoli commentedMar 23, 2016
👍 however I think the tests need to be reverted too to ensure that we are fixing the problem (see#14403 (comment)) |
HeahDude commentedMar 23, 2016
@mnapoli Thanks for the review, the tests are already reverted (otherwise they would fail). Failures are unrelated. |
xabbuh commentedMar 25, 2016
You will now need to update the lowest version of the Form component required by the FrameworkBundle to |
HeahDude commentedMar 25, 2016
@xabbuh so it should be done in a second commit since this one is supposed to be reverted in 3.0, right ? |
xabbuh commentedMar 25, 2016
Not necessarily if I don't misunderstand you. We need to revert all the changes here in the |
HeahDude commentedMar 25, 2016
Yes we do. |
HeahDude commentedMar 25, 2016
Thanks@xabbuh tests are all green now (excepted appveyor being unusually long). |
HeahDude commentedMar 25, 2016
It seems appveyor has bugged, why does it still try to install icu v51 ? |
xabbuh commentedMar 26, 2016
👍 Status: Reviewed |
HeahDude commentedMar 26, 2016
@xabbuh Do I need to upgrade the |
xabbuh commentedMar 26, 2016
@HeahDude Usually you would have to do that. But this time that's not necessary as this was apparently done in another pull request before:https://github.com/symfony/symfony/blob/2.8/src/Symfony/Bridge/Twig/composer.json#L25 |
HeahDude commentedMar 26, 2016
@xabbuh Thanks for your leading :) |
fabpot commentedMar 27, 2016
Thank you@HeahDude. |
This PR was merged into the 2.8 branch.Discussion----------[Form] Fix BC break introduced in#14403| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | n/a| License | MIT| Doc PR | -Commits-------5f48c6a [Form] Fix BC break introduced in#14403
mnapoli commentedMar 27, 2016
Thanks@HeahDude for the fix! |
HeahDude commentedMar 27, 2016
@mnapoli Thanks for reporting this issue :) |
* 3.0: bumped Symfony version to 2.8.5 updated VERSION for 2.8.4 updated CHANGELOG for 2.8.4 Revert "bug#18275 [Form] Fix BC break introduced in#14403 (HeahDude)" improved comment [FileSystem] Add support for Google App Engine. [2.8] fix mocks bumped Symfony version to 2.7.12 [Form] Fix BC break introduced in#14403 updated VERSION for 2.7.11 updated CHANGELOG for 2.7.11 [Request] Fix support of custom mime types with parameters fix mocks fix mocks [Validator] do not treat payload as callback