Skip to content
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

Provide PHP 8.0 support #18

Merged
merged 2 commits into from
Jun 9, 2021
Merged

Provide PHP 8.0 support #18

merged 2 commits into from
Jun 9, 2021

Conversation

fezfez
Copy link
Contributor

@fezfez fezfez commented Feb 8, 2021

  • Modify composer.json to provide support for PHP 8.0 by adding the constraint ~8.0.0
  • Modify composer.json to drop support for PHP less than 7.3
  • Modify composer.json to implement phpunit 9.3 which supports PHP 7.3+
  • Modify .travis.yml to ignore platform requirements when installing composer dependencies (simply add --ignore-platform-reqs to COMPOSER_ARGS env variable)
  • Modify .travis.yml to add PHP 8.0 to the matrix (NOTE: Do not allow failures as PHP 8.0 has a feature freeze since 2020-08-04!)
  • Modify source code in case there are incompatibilities with PHP 8.0

I have also moved to "laminas/laminas-coding-standard": "~2.1.0"

tests are working in php 8 with --ignore-platform-reqs

@fezfez
Copy link
Contributor Author

fezfez commented Feb 8, 2021

Note : i think the ci with travis ci is deprecated and laminas org is moving to github action.
i can directly move to github action in this PR if there is a guide to migrate

@fezfez
Copy link
Contributor Author

fezfez commented Feb 8, 2021

next will be to drop laminas/console before release ?

@fezfez
Copy link
Contributor Author

fezfez commented Mar 26, 2021

@weierophinney ping 😃

@froschdesign froschdesign linked an issue Mar 26, 2021 that may be closed by this pull request
6 tasks
@froschdesign
Copy link
Member

@fezfez

i can directly move to github action in this PR if there is a guide to migrate

A separate pull request is easier to review. All required steps can be found in this pull request: laminas/laminas-server#26

Thanks in advance! 👍

@fezfez fezfez mentioned this pull request Mar 26, 2021
@fezfez
Copy link
Contributor Author

fezfez commented Mar 26, 2021

@froschdesign : see #19

@FabianKoestring
Copy link
Contributor

Are there any changes to be made here?

@froschdesign
Copy link
Member

@FabianKoestring
Please have a look at #19. Some help is needed there. Add your ideas or a review.
Thanks in advance! 👍

@FabianKoestring
Copy link
Contributor

@FabianKoestring
Please have a look at #19. Some help is needed there. Add your ideas or a review.
Thanks in advance! 👍

Looks like PR (#28) is ready .

@fezfez
Copy link
Contributor Author

fezfez commented Jun 1, 2021

@Xerkus : this PR seem ok to me, the psalm job fail but is also fail in 3.5.X and this is not related to this PR..

@Xerkus
Copy link
Member

Xerkus commented Jun 1, 2021

Oof. Psalm merge did not go as planned.

@Xerkus
Copy link
Member

Xerkus commented Jun 1, 2021

@fezfez lock file should be made on/for php 7.4
Looks like you did it on different version which is what causes phpcs and psalm to fail

@Xerkus
Copy link
Member

Xerkus commented Jun 1, 2021

Dropping laminas-console would require a major release.
This is not a production code, so we can potentially drop it from dependenecies and move to suggests. But it is not a route I personally would take.

@fezfez
Copy link
Contributor Author

fezfez commented Jun 1, 2021

Dropping laminas-console would require a major release.
This is not a production code, so we can potentially drop it from dependenecies and move to suggests. But it is not a route I personally would take.

Yeah, i think it's a non sens to add PHP 8 support without dropping laminas-console, i've droped it in last commit but this PR would require a major version.

@Xerkus
Copy link
Member

Xerkus commented Jun 1, 2021

@fezfez what we will have to do is split this into two parts. Pull everything else, except for enabling php 8, into a separate PR for 3.5. php 8 + dropped console would then target 4.0

@fezfez fezfez force-pushed the php8 branch 2 times, most recently from 9809df7 to 2d5d6d0 Compare June 2, 2021 06:00
@fezfez
Copy link
Contributor Author

fezfez commented Jun 2, 2021

@Xerkus :

@fezfez what we will have to do is split this into two parts. Pull everything else, except for enabling php 8, into a separate PR for 3.5. php 8 + dropped console would then target 4.0

it's done :)

@fezfez fezfez force-pushed the php8 branch 2 times, most recently from 49dacb2 to 15f7a14 Compare June 2, 2021 06:49
Signed-off-by: Stéphane Demonchaux <[email protected]>
composer.json Outdated
Comment on lines 24 to 25
"php": "^7.3 || ~8.0.0",
"laminas/laminas-console": "^2.6",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop 8.0 in this PR as well, since it is not installable with laminas-console

That won't matter for composer but It will make clear for users that they need to upgrade to 4.0 for php 8

Copy link
Contributor Author

@fezfez fezfez Jun 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the same strategy than laminas-cache can be used in this case. example

in composer.json

    "replace": {
        "llaminas/laminas-console": "*"
    }

in this way, 3.5.x can be use with php 8, dont you think it wort it ?

Edit: never mind, it cant work because the autoload will break...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see a value in this. It is a dev dependency kind of package. Something else depending on it is quite unlikely.

@Xerkus
Copy link
Member

Xerkus commented Jun 2, 2021

@fezfez are you in our slack? I would like to request some chores for 3.5 before we move on to 4,0

@Xerkus Xerkus self-assigned this Jun 2, 2021
Signed-off-by: Stéphane Demonchaux <[email protected]>
@weierophinney weierophinney changed the title fix #15 Provide PHP 8.0 support Jun 9, 2021
Copy link
Member

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@weierophinney weierophinney merged commit 5eb5f91 into laminas:3.5.x Jun 9, 2021
@fezfez fezfez deleted the php8 branch September 20, 2021 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP 8.0 support
5 participants