Skip to content

SF4 and php7 mods #13

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Feb 18, 2018
Merged

SF4 and php7 mods #13

merged 6 commits into from
Feb 18, 2018

Conversation

w473
Copy link
Contributor

@w473 w473 commented Feb 13, 2018

Modification to make it work with php7 and symfony 4
NOT TESTED with previous versions

@redthor
Copy link
Contributor

redthor commented Feb 13, 2018

Thanks @w473

We're running php7.1 here but we don't have your addition:

+    "config": {
+        "platform": {
+            "ext-mongo": "1.6.16"
+        }

We do have this in our main app composer.json:

    "provide" : {
        "ext-mongo": "*"
    },

perhaps that's all you need for php7?

Could you submit another PR with just the sf4 changes or change this one to have just sf4?

If you think we still need php7 changes, please submit another PR just for that so we can review separately.

Thanks again for helping.

@w473
Copy link
Contributor Author

w473 commented Feb 14, 2018

I will check this evening your suggestions. But to make it work I need info about ext-mongo for php7 and I need php7.1 for sf4

@w473
Copy link
Contributor Author

w473 commented Feb 14, 2018

So I've rechecked everything and it looks like its required to install mapper for old mongo lib:
alcaeus/mongo-php-adapter
so I updated readme file, and removed wrong composer.json entries

@caciobanu
Copy link
Member

The build is failing because of this : if [ "x${ADAPTER_VERSION}" != "x" ]; then composer require "alcaeus/mongo-php-adapter=${ADAPTER_VERSION}" --ignore-platform-reqs; fi. We have to update the build script.

@redthor redthor mentioned this pull request Feb 15, 2018
@redthor
Copy link
Contributor

redthor commented Feb 15, 2018

I've opened #14 to remove the php5.3 build error.

And then opened #15 to remove the php7.0 error.

The #15 build is passing but I've removed support for php7.0 and jumped to php7.1

If we go with those changes then we can see how this build is going after that.

@redthor
Copy link
Contributor

redthor commented Feb 16, 2018

@w473 do you mind just updating the PR with master so we can get the travis tests running, then we should be good to go

@w473
Copy link
Contributor Author

w473 commented Feb 16, 2018

still issues...:

5.3 is not pre-installed; installing
Downloading archive: https://s3.amazonaws.com/travis-php-archives/binaries/ubuntu/12.04/x86_64/php-5.3.tar.bz2
370.45s$ curl -s -o archive.tar.bz2 $archive_url && tar xjf archive.tar.bz2 --directory /
0.00s
0.02s$ phpenv global 5.3
rbenv: version `5.3' not installed
The command "phpenv global 5.3" failed and exited with 1 during .

@caciobanu
Copy link
Member

We should just ignore 5.3 and drop builds for it as Symfony 4 does not even install on PHP 5.3.

@w473
Copy link
Contributor Author

w473 commented Feb 16, 2018

well this should work with older version of symfony, but supporting now anything below php 5.6 is somehow strange for me

@redthor
Copy link
Contributor

redthor commented Feb 18, 2018

I've made release 1.2 that people on php5.3 (if there are any!) can use (that build passed).

We can merge this one into master.

Then we can remove php5.3 and also look at removing php5.* and maybe symfony 2.*?

@redthor redthor merged commit cfff17a into doesntmattr:master Feb 18, 2018
@redthor
Copy link
Contributor

redthor commented Feb 18, 2018

On merge this PR's build has passed

@redthor
Copy link
Contributor

redthor commented Feb 18, 2018

#16 opened to remove php5.3 anyway

@redthor
Copy link
Contributor

redthor commented Feb 19, 2018

Thanks for helping @w473

I've tagged a 1.2.1 release - can you try that out?

@w473
Copy link
Contributor Author

w473 commented Feb 19, 2018

thx, I will check it tomorrow

@w473
Copy link
Contributor Author

w473 commented Feb 20, 2018

works properly THX!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants