rollerworks / PasswordStrengthBundle

Symfony Password strength and blacklisting validator bundle
MIT License
142 stars 26 forks source link

Not compatible with Validation API 2.5 #37

Closed hlecorche closed 9 years ago

hlecorche commented 9 years ago

This bundle is not compatible with Validation API 2.5. It would have to be compatible with 2.4 and 2.5 versions.

sstok commented 9 years ago

You are right, can you provide a PR? Detecting if the 2.5 API is used can be done by checking the interface of the Context.

hlecorche commented 9 years ago

But this change requires Symfony 2.5 Is it a problem?

And why doest BlacklistValidationTest extend PHPUnit_Framework_TestCase and not AbstractConstraintValidatorTest ? (AbstractConstraintValidatorTest uses a useful system to test 2.4; 2.5 and 2.5BC API)

sstok commented 9 years ago

I'm OK with bumping the minimum version to 2.3 :+1: which is actually something I already doing with the other projects.

We can keep compatibility with 2.3, $this->context will give the current context object. You can use a simple instanceof check to see if the legacy (2.3 compatible) or the new 2.4, and execute a separate logic per version.

The BlacklistValidationTest was introduced back in 2013 ;) there was no AbstractConstraintValidatorTest back-then.

hlecorche commented 9 years ago

I agree with you but when I say that this change requires Symfony 2.5, the requirement is for tests and not for validators (since Symfony 2.5, AbstractConstraintValidatorTest can test with 2.4, 2.5 and 2.5BC API with getApiVersion() method)

This requirement can be added to the require-dev section.

sstok commented 9 years ago

AbstractConstraintValidatorTest is now also available for 2.3, with the same API as the AbstractConstraintValidatorTest in 2.4 :+1: (expect for getApiVersion() but thats no problem).

So we should be able to run tests with 2.3 and AbstractConstraintValidatorTest.

hlecorche commented 9 years ago

Ok, I'll make the changes

sstok commented 9 years ago

Can you merge #38 into you're pull request?

hlecorche commented 9 years ago

Ok