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

[shopsys] Updated PHP to version 7.3 #694

Merged
merged 2 commits into from Jan 24, 2019
Merged

[shopsys] Updated PHP to version 7.3 #694

merged 2 commits into from Jan 24, 2019

Conversation

grossmannmartin
Copy link
Member

Q A
Description, reason for the PR Support new version of PHP
New feature No
BC breaks No
Fixes issues ...
Standards and tests pass Yes
Have you read and signed our License Agreement for contributions? Yes

@grossmannmartin
Copy link
Member Author

Currently, we have to install composer dependencies with --ignore-platform-reqs, because FriendsOfPHP/PHP-CS-Fixer does not support PHP 7.3.
There is an issue about this topic.

build.xml Outdated Show resolved Hide resolved
@vitek-rostislav
Copy link
Contributor

vitek-rostislav commented Dec 27, 2018

Currently, we have to install composer dependencies with --ignore-platform-reqs, because FriendsOfPHP/PHP-CS-Fixer does not support PHP 7.3.
There is an issue about this topic.

@TomasVotruba, what do you think about this, please?

It seems a little bit dangerous to me - when we want to say that we support php 7.3, we should not use tools that lack the support. And by ignoring the platform requirements, we could overlook some incompatibility problems, right?

Btw, should not the dependency on symplify/easy-coding-standard be defined in require-dev?

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Dec 27, 2018

It's has nothing to do with code. php-cs-fixer runs on PHP 7.3 with no problem.
It's missuse of composer to force people to work, see https://stackoverflow.com/a/53727287/1348344

Btw, should not the dependency on symplify/easy-coding-standard be defined in require-dev?

What line do you talk about? Why?

@boris-brtan boris-brtan changed the title WIP [shopsys] Updated PHP to version 7.3 [shopsys] Updated PHP to version 7.3 Dec 27, 2018
Copy link
Contributor

@boris-brtan boris-brtan left a comment

Choose a reason for hiding this comment

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

Hi, thx for the PR, i left just some suggestions.
I tested this on frontend and in administration and it seems to be working well from the users point of view.
There is also one in rv message above.

UPGRADE.md Outdated Show resolved Hide resolved
@vitek-rostislav
Copy link
Contributor

Hi @TomasVotruba, thanx for your reply 👍

btw, should not the dependency on symplify/easy-coding-standard be defined in require-dev?

What line do you talk about? Why?

My bad...I was thinking about shopsys/coding-standards which is in require-dev indeed. The symplify/easy-coding-standard is defined in require section in monorepo only so everything is ok 🙂

@grossmannmartin grossmannmartin added the Status: on hold Issue cannot be resolved right now or is waiting for something label Jan 3, 2019
@grossmannmartin
Copy link
Member Author

I simplified this PR, so it covers only one thing - Upgrade to PHP 7.3.
We decided, that we don't want to install dependencies with --ignore-platform-reqs option, so I'm putting this on hold until PHP-CS-Fixer/PHP-CS-Fixer#3697 (PR PHP-CS-Fixer/PHP-CS-Fixer#4184) will be solved.

Memory limit of PhpStan or possible upgrade of it should be solved in an independent PR, thank you for your insights on this topic 👍

@keradus
Copy link

keradus commented Jan 6, 2019

FYI @grossmannmartin : PHP CS Fixer v2.14 supports PHP 7.3

@MattCzerner
Copy link
Contributor

Seems like its done bois. PHP-CS-Fixer/PHP-CS-Fixer#3697 (comment)

@LukasHeinz LukasHeinz removed the Status: on hold Issue cannot be resolved right now or is waiting for something label Jan 16, 2019
@grossmannmartin grossmannmartin self-assigned this Jan 16, 2019
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.

None yet

7 participants