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

IBX-784: Provided PHP8 compatibility for Selection FieldType #3112

Merged
merged 4 commits into from
Jul 21, 2021

Conversation

ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Jul 16, 2021

Question Answer
JIRA issue https://issues.ibexa.co/browse/IBX-784
Bug/Improvement yes
New feature no
Target version 7.5
BC breaks no
Tests pass yes
Doc needed no

It seems that there is some unintended BC break in php8 related to https://wiki.php.net/rfc/named_params.

QA:
Changes were made around multilingual selection options, so that is the part thats need to be tested if we push this thru QA.

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@ViniTou ViniTou requested review from adamwojs, Steveb-p, alongosz and a team July 16, 2021 11:16
Copy link
Contributor

@Steveb-p Steveb-p left a comment

Choose a reason for hiding this comment

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

Just one nitpicky comment you might want to check, optionally.

eZ/Publish/Core/FieldType/Selection/Type.php Outdated Show resolved Hide resolved
@Steveb-p Steveb-p requested a review from a team July 16, 2021 11:26
@tomaszszopinski tomaszszopinski self-assigned this Jul 20, 2021
Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on eZPlatform 2.5-dev with diff.

@alongosz alongosz merged commit ce61bd9 into 7.5 Jul 21, 2021
@alongosz alongosz deleted the php8-user-func-type branch July 21, 2021 11:56
@alongosz
Copy link
Member

@ViniTou you can merge it up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants