[3.0] [Validator] deprecations cleanup #15708
Conversation
|
Some code is comented out. It should be removed or fixed :) |
| // )); | ||
| // | ||
| // $this->assertTrue(is_callable($form->getConfig()->getOption('validation_groups'))); | ||
| // } |
There was a problem hiding this comment.
these comments must be reverted (any non legacy test should be kept)
There was a problem hiding this comment.
sorry, forgotten testing
|
@jakzal Thanks. I forgot it there, should be ready now. All tests pass now, except 2.8 build. Btw, why it's there this Travis build for 3.0 branch? |
|
The tests on 2.8 are for verifying that a PR on 3.0 does not break 2.8. This means you still have something to fix :) That can be by doing an other PR on 2.8 to exclude validator ~3.0.0 from form's composer.json. Or by figuring out the failure and finding an other, less radical, patch on 2.8 to make it compatible with your PR (which may not be possible). |
|
@nicolas-grekas This is my first bigger PR, so I'd rather go with:
Would that be ok? |
| $validator = $this->getMock('Symfony\Component\Validator\Validator\ValidatorInterface'); | ||
|
|
||
| $listener = new ValidationListener($validator, $this->violationMapper); | ||
| $this->assertAttributeSame($validator, 'validator', $listener); | ||
| } | ||
|
|
||
| public function testValidatorInterfaceUntilSymfony24() |
There was a problem hiding this comment.
this test should be @group legacy in 2.8
|
@TomasVotruba excluding 3.0 is an option only when there is no other option :) |
This PR was merged into the 2.8 branch. Discussion ---------- [Validator] mark test as legacy | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Ref #15708 (comment) Failing test are hhvm binaries: https://travis-ci.org/symfony/symfony/jobs/79951993#L530 Commits ------- cf18a99 [Validator] mark test as legacy
|
rebase needed @TomasVotruba |
|
Mmm, rebased. There are some some new commits that keeps breaking it. Only 2.8 was failing, now is every branch. I don't have such a deep experience with Validator, when error message would help me to fix it. I'm thinking of splitting to smaller PRs, that can be merged at once. What do you thing? |
|
@TomasVotruba If you think it would be easier to manage smaller PRs, go for it. |
|
I'm giving it a try in #16024 |
|
@fabpot Thanks, I got busy recently. |
Please review.