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

More Match class PHP 8 fun (in 6.x) #1978

Closed
reedy opened this issue Sep 16, 2021 · 5 comments
Closed

More Match class PHP 8 fun (in 6.x) #1978

reedy opened this issue Sep 16, 2021 · 5 comments

Comments

@reedy
Copy link
Contributor

reedy commented Sep 16, 2021

From our downstream issue - https://phabricator.wikimedia.org/T268861#7359008

Because of how I did MatchQuery extends Match, Match is still executed, meaning it does fail on PHP 8.0. And that's kinda fine, as I did it for forward compat purposes (so people could switch to MatchQuery earlier, and depending on their code, be able to support later 6.x and also 7.x).

As per https://github.com/ruflin/Elastica/blob/6.1.5/composer.json#L15, 6.1.x doesn't support PHP 8 either (at least officially, you can obviously ignore certain requirements).

We're contemplating how to do PHP 8 migration for Wikipedia (et al), and as our ES upgrade might not have happened by then (and hence, also upgrading ruflin/elastica and elasticsearch/elasticsearch-php).

Would a patch to make MatchQuery the canonical (ie find and replace Match with MatchQuery, and then make the class Match extend MatchQuery instead) be accepted?

This is related to #1913... And would be needed to support that. As per the backtrace below (from the task/comment linked above)

[99a80ba4319b973cca1c54a7] /index.php?search=%C3%A9v%C3%A9nement+%3A+c&title=Sp%C3%A9cial%3ARecherche&go=Lire&ns0=1 ParseError: syntax error, unexpected token "match"

Backtrace:

from /var/www/vhosts/fallout-wiki.com/httpdocs/extensions/Elastica/vendor/ruflin/elastica/lib/Elastica/Query/MatchQuery.php(10)

And that line is https://github.com/ruflin/Elastica/blob/6.1.5/lib/Elastica/Query/MatchQuery.php#L10, so we would need to stop that being executed.

I think I did mumble about this in one of the prior tasks, expecting something like this might eventually happen.

So I guess the TLDR... Is would a patch swapping MatchQuery and Match be welcomed, in support of #1913 and potentially being able to make 6.x support PHP 8 (obviously might be other issues down the line).

@reedy reedy changed the title More Match class PHP 8 fun More Match class PHP 8 fun (in 6.x) Sep 16, 2021
@Hikariii
Copy link

+1 for php8 support on the 6.x branch.

@Hikariii
Copy link

#1913

@reedy
Copy link
Contributor Author

reedy commented Sep 22, 2021

Seems #1977 was created before I created this issue...

@ruflin
Copy link
Owner

ruflin commented Sep 23, 2021

I definitively support this. The important part for me is that we don't break existing installations but it seems with this approach this is possible.

@reedy
Copy link
Contributor Author

reedy commented Sep 23, 2021

My version in #1981 does that, AFAIK...

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

No branches or pull requests

3 participants