Symfony 3 Compatibility

[pmeulen]

After updating composer.json, composer.lock should be updated as well. It is not in the PR however.

[ghost]

IMO: We should fully remove the composer.lock file as I explained here: https://github.com/SURFnet/yubikey-api-client-bundle/issues/6#issuecomment-166599885

[DRvanR]

The symfony packages don't disallow PHP8+, should we really do that?

The fact that others may not disallow PHP > 7 doesn't mean we should. Having discussed this yesterday however, we would like to propose to drop PHP 5.4 and 5.5 support:

PHP 5.4 is already EoL and should no longer be supported going forward PHP 5.5 is security fixes only, we would like to drop the PHP 5.5 support PHP 5.6 is old-stable, but should still be supported since 7.0 has only been out for 20 days PHP 7.0 is current-stable and should be supported.

Thoughts?

As an aside: this would mean a BC break, so we'll have to release a new major. Nothing big, but we have to make sure it happens :smile:

With respect to the Symfony3 support, I am unsure it is 3.0 compatible. It would have to be checked against the UPGRADING.md from Symfony. Until it has been verified to be compatible I'm hesitant to support 3.0.

[ghost]

Travis CI has not checked it against Symfony 3. It has downloaded Symfony 2.7.3 (not even 2.8) over composer. Any idea why? We need to ensure hat Travis CI will check it against Symfony 3.0 to know if any test fails.

[DRvanR]

Yes, because of the lock file.

We need to ensure hat Travis CI will check it against Symfony 3.0 to know if any test fails.

That may result in false positives, due to lack of coverage, and no tests for integrations. Furthermore quite a bit of non-functionals (e.g. factory services definition etc) have changed between SF2 and SF3. Just relying on the CI is not enough to ensure proper SF3 support, hence why it should not just be added.

[ghost]

@DRvanR Sure, we should check every line of code manually :+1: But it would be interesting why it does not check against Symfony 2.8 or 3.0 - is there any dependency which is not updated to 2.8 or 3.0+?

[DRvanR]

The fact that it doesn't use 2.8 or 3.0 is due to the lock file :wink:

[ghost]

@DRvanR The Travis CI build has passed. I cannot find any code that is not compatible with Symfony 3 because this bundle has not much specific code for Symfony - it is mostly own code for the Yubikey features. What do you think - is anything missing?

[DRvanR]

A lot may be missing, for instance the service definitions need to be updated to be yaml compliant. This is just a simple example that I knew from the top of my head. Hence why it needs proper verification to be 3.0 compliant, it is much more than just running a testsuite due to non-code or integration issues that may only be discovered when testing in 3.0.

Good change on the PHP version constraint btw!

[ghost]

I've updated the service definition file. I also checked the code with the SensioLabs DeprecationDetector but it has not found any violations.

If you find anything that should be updated to Symfony 3 just let me know. Currently I do not have any application on Symfony 3 that use this bundle, maybe anybody else could check it against a real application.

[quentinus95]

Please merge this pull request.

[ghost]

@quentinus95 Please check this PR on a real application. Does it work fine? We need to confirm that this PR is working without any issues.

[quentinus95]

Yes, everything is fine !

[ghost]

@DRvanR Please merge it :+1:

[DRvanR]

Hope to merge this tomorrow, otherwise Wednesday. Will coincide with moving the client itself to a different repo (as discussed in #16 ). This will lead to a new 2.0 version.

[quentinus95]

Any news ? :smile:

[rjkip]

Hi @quentinus95, we are quite busy ;) I will try to address #16 later today.

[quentinus95]

Thanks for the merge !

[rjkip]

Hi all, I just released version 2.0.0, which includes these changes.

[quentinus95]

Thanks ! I can move to SF3 now :smiley:

[ghost]

@rjkip Could you please fix #19 ? Thanks! :)