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

Allow sury repo for PHP >= 7.1. #555

Closed
wants to merge 1 commit into from

Conversation

Hexta
Copy link

@Hexta Hexta commented Dec 10, 2019

Pull Request (PR) description

Allow sury repo for PHP >= 7.1.

This Pull Request (PR) fixes the following issues

@Hexta
Copy link
Author

Hexta commented Feb 8, 2020

Could anybody please take a look at the PR?

@@ -67,7 +67,13 @@
}
}

if ($sury and $php::globals::php_version in ['5.6','7.1','7.2'] ) {
Copy link
Member

Choose a reason for hiding this comment

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

would be better to simply add the supported php version of sury in the array, only this way non valid php versions could be catched.
with your change a php version like 8.0 would be allowed

Copy link
Author

Choose a reason for hiding this comment

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

Do you really think that hardcoded list of supported php versions is a good idea?

Copy link
Member

Choose a reason for hiding this comment

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

not really, i prefer to have them configureable via hiera with sane default values directly set in this module with hiera. i would prefer that for many hardcoded values (like the repo key id) but without a bigger refactoring this is not possible.

to simply remove the version check feels wrong to me.

@bastelfreak
Copy link
Member

@Hexta please rebase against our latest master branch.

@vox-pupuli-tasks
Copy link

Dear @Hexta, thanks for the PR!

This is Vox Pupuli Tasks, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

@Hexta Hexta closed this Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants