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

Older PHP does not support const arrays. #170

Closed
wants to merge 2 commits into from
Closed

Older PHP does not support const arrays. #170

wants to merge 2 commits into from

Conversation

GlazerMann
Copy link
Collaborator

Probably better to update PHP on Wiki Tools to match test environment.

@GlazerMann
Copy link
Collaborator Author

Seems to me that the test environment might need to match the Wiki PHP version.

@ms609
Copy link
Owner

ms609 commented Sep 21, 2017

Oh dear - I'd assumed (hoped) that wmflabs were running PHP7. The code will require some significant refactoring otherwise - you can't define arrays either, so we'd have to go back to using global variables (strongly discouraged). Can we explore what it would take for wmflabs to upgrade to php7? It might be as simple as re-naming files to the .php7 suiffix, if they have that version installed

@GlazerMann
Copy link
Collaborator Author

It is 5.5.9.

Old 👵🏻👴🏻

@ms609
Copy link
Owner

ms609 commented Sep 22, 2017

That's frustrating. What is the prospect of them upgrading to PHP7? For now, I've added 5.5.9 to the Travis test cases, but it will take some work to re-adapt the code; probably the thing to do is to make a public function const_array($name) that returns the array required, and replace all calls to the constant arrays with a call to the function. I've no time to do that myself at the moment; feel free to have a shot yourself if that sounds easier than pushing for a PHP version upgrade at WMFlabs!

@GlazerMann
Copy link
Collaborator Author

Fighting this issue in wiki land

@GlazerMann
Copy link
Collaborator Author

5.6 is supported sort of. Check Wikipedia talk page.

@ms609
Copy link
Owner

ms609 commented Sep 27, 2017

I believe that PHP7 is the first version to allow const arrays.

@GlazerMann
Copy link
Collaborator Author

Actually 5.6 supports it. It appears that it was considered useful enough to add. That’s the last pre-7 version.

@ms609
Copy link
Owner

ms609 commented Sep 27, 2017 via email

@GlazerMann
Copy link
Collaborator Author

https://wikitech.wikimedia.org/wiki/Help:Toolforge/Kubernetes#Versions_.26_Packages

Yeah it means using a container. It’s not just a flag. This discussion is probably better on Wikipedia

@kaldari
Copy link
Contributor

kaldari commented Sep 27, 2017

@GlazerMann: Something seems to be messed up with the Kubernetes set up on Toolforge. I haven't been able to get it running at all.

@kaldari
Copy link
Contributor

kaldari commented Sep 27, 2017

@GlazerMann: Finally managed to get it working (I think). See if the dev code works now.

@GlazerMann
Copy link
Collaborator Author

The dev code does not work as far as I can tell. What types of errors do you get. As a user, we do not see the erros

@GlazerMann
Copy link
Collaborator Author

Once we have the error messages; we can fix PHP Code to work with 5.6

@ms609
Copy link
Owner

ms609 commented Sep 29, 2017

Even in PHP 5.6, constants must evaluate to scalar values: see
https://travis-ci.org/ms609/citation-bot/jobs/281183252

Looks like the code will have to be refactored. This might be something I can have a bash at today.

@ms609
Copy link
Owner

ms609 commented Sep 29, 2017

Please direct any further discussion on this topic to the related issue.

@ms609 ms609 closed this Sep 29, 2017
@GlazerMann GlazerMann deleted the patch-23 branch September 29, 2017 13:09
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

3 participants