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

Update Dockerfile #455

Merged
merged 1 commit into from
Oct 24, 2020
Merged

Update Dockerfile #455

merged 1 commit into from
Oct 24, 2020

Conversation

t-h-e
Copy link
Contributor

@t-h-e t-h-e commented Oct 20, 2020

upgrade base image to php 7.4.11
as well as necessary dependencies
to be able to create a docker image for lansuite again

@andygrunwald
Copy link
Collaborator

Hey @t-h-e,
thanks a lot for this pull request.
The changes are looking good and I would love to merge this. However, jumping from PHP 7.0 to PHP 7.4 might not be as straightforward.
Especially in LanSuite, this source is quite old and I am not 100% sure if it runs as expected on PHP 7.4.
That is the reason why I am not hitting the merge button immediately.

@M4LuZ What do you think?

@t-h-e
Copy link
Contributor Author

t-h-e commented Oct 21, 2020

No worries. I should have mentioned that I have not tested it further than just checking if the lansuit setup starts.
I'm trying to migrate from a rather old lansuite version with a few minor changes to the current version. While also migrating the changes. If the changes still seem useful, I will create pull requests for them.
Anyway I will have to test everything after migrating anyway, so for me it's also fine to leave the pull request here until further confirmation that lansuite works properly with php 7.4
At least here the change is already accessible to everyone 😃

@M4LuZ
Copy link
Collaborator

M4LuZ commented Oct 21, 2020

I would propose to change dockerfile to PHP 7.2 at first (as that is fairly certain to work) and upgrade travis to build against 7.2 -7.4.
If we don't get any complaints from there we should give 7.4 a shot and update version then

@t-h-e
Copy link
Contributor Author

t-h-e commented Oct 22, 2020

7.2 is fine for me as well for now, but keep in mind that 7.2 is nearly end of life as well.
see https://www.php.net/supported-versions.php

@M4LuZ
Copy link
Collaborator

M4LuZ commented Oct 22, 2020

That is well known and understood.
Reason why I specifically mention PHP 7.2 is because that is the highest version we test against in Travis and thus had at least some test coverage by the CI-System.
I agree that we should move on to 7.4 ASAP and only see that as stop-gap measure till then.

upgrade base image to php 7.2.34
@M4LuZ M4LuZ merged commit 350e3bb into lansuite:master Oct 24, 2020
M4LuZ added a commit that referenced this pull request Dec 3, 2020
… Travis (wrong method name applied in PR #458)

refs #455
fixes #461
andygrunwald pushed a commit that referenced this pull request Dec 3, 2020
… Travis (wrong method name applied in PR #458)

refs #455
fixes #461
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