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

Add missing versions, use proper php version and add erase / install database parameters #226

Merged
merged 1 commit into from
Sep 9, 2020
Merged

Conversation

PierreRambaud
Copy link
Contributor

@PierreRambaud PierreRambaud commented Sep 2, 2020

Questions Answers
Description? Add missing images, use the correct PHP version on recent images. Add PS_INSTALL_DB parameter
Also now build image for pre-release builds as was asked in this core issue PrestaShop/PrestaShop#20563
Type? improvement
BC breaks? no
Deprecations? no
How to test? Build must pass

versions17.txt Show resolved Hide resolved
fi
if ((10#${ver1[i]} > 10#${ver2[i]}))
then
return 1
Copy link
Contributor

Choose a reason for hiding this comment

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

The return 1|2 is a bit weird no? Usually for comparison functions you return 1, -1 or 0 when equal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only in PHP, it depends on the language :p

Copy link
Contributor

Choose a reason for hiding this comment

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

That's clearly not only in PHP but doesn't matter ^^ You're right that conventions differ from language to language

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And this part of code came from stackoverflow. I plan to refactor this script in a proper language like python in order to make it more customizable and manage correctly PHP and PS versions.

@@ -123,3 +123,6 @@
1.7.6.4
1.7.6.5
1.7.6.7
1.7.6.6
1.7.6.7
1.7.7.0-beta.2
Copy link
Contributor

Choose a reason for hiding this comment

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

How come we add the beta build this time? Seems like we never did it for previous versions 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It creates the Dockerfile for the beta version, but we must push it manually for the moment.
I plan to refactor this repository in order to manage property PHP / PrestaShop version.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm, but we didn't push any other beta, so why this one?
Unless we clean them when the official release is ready?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a new demand from the community, they created an issue weeks ago. We will do it for all our versions, beta, alpha, RC, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

So it's in addition with the already existing beta build? This one is built on the branch last nighly build though so I guess the community needs a Docker that matches the beta tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PrestaShop/PrestaShop#20563 They are also able to use the nightly build, but for testing usage, it's also better to have access to the same beta build we pushed on the dot com.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok nice improvement indeed! I'll just add the issue in the description to understand the new need

Also we might need to update the release doc, because now we will have to update the docker images for each pre-release build

@jolelievre jolelievre merged commit 652719b into PrestaShop:master Sep 9, 2020
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.

5 participants