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

Fix array_walk() call on PHP 8 #1095

Closed
wants to merge 1 commit into from

Conversation

derrabus
Copy link

@derrabus derrabus commented Jan 8, 2021

Running version 6.7 of this library on PHP 8 will result in a warning being triggered:

Argument #2 ($key) must be passed by reference, value given

The fix is fairly trivial because the affected variable is not used and can be removed.

This issue has already been reported as #1085 fixed on higher branches. However the project I'm working with at the moment is currently unable to upgrade to Elasticsearch 7. And as far as I can tell, this issue seems to be our only blocker for upgrading to PHP 8.

If the 6.7.x branch is closed for bugfixes, please close this PR.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@alexander-schranz
Copy link
Contributor

The ones which did subscribe this PR. Elasticsearch did release a PHP 8 supported Version with 7.11: https://github.com/elastic/elasticsearch-php/releases/tag/v7.11.0 not sure if they will support PHP 8 with older version maybe @ezimuel can make a statement about it.

@ezimuel
Copy link
Contributor

ezimuel commented Feb 12, 2021

The original plan was to support PHP 8 only for elasticsearch-php 7.x (I just released it yesterday with 7.11.0). Considering that elasticsearch 6.x is still quite popular I can consder to add the support of PHP 8 al well.

@derrabus can you send the PR to 6.x? Thanks!

@ezimuel ezimuel added this to the 6.8.0 milestone Feb 12, 2021
@derrabus derrabus changed the base branch from 6.7.x to 6.x February 12, 2021 15:13
@derrabus
Copy link
Author

Sure, no problem. I've changed the target branch and rebased the changes.

If you need more help building a PHP8-compatible 6.x release, let me know.

@derrabus
Copy link
Author

The Travis failures are unrelated to my changes. I'm trying to fix them in #1105.

@derrabus
Copy link
Author

I think we can close the PR because the problem has been addressed in b316710. I've switched our project to the 6.8.x branch of this library and it works like a charm!

Thank you very much, @ezimuel!

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.

4 participants