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

Changed checkFileSystemPermissions arguments so PHP 8 won't throw Deprecated info #404

Merged
merged 1 commit into from
Feb 9, 2022

Conversation

mateuszdebinski
Copy link
Contributor

If we have set opcache.enable = 1 with PHP 8, then information is displayed: "Deprecated: Optional parameter $path declared before required parameter $permissions is implicitly treated as a required parameter in /var/www/html/vendor/tedivm/stash/src/Stash/Utilities.php on line 212", it causes a problem with being able to set via ini_set function session.path:

Warning: ini_set(): Session ini settings cannot be changed after headers have already been sent

The proposed change will not affect the operation of the application and will remove information about Deprecated, which will prevent the appearance of Warning

@tedivm tedivm merged commit 4f127e7 into tedious:master Feb 9, 2022
@mateuszdebinski mateuszdebinski deleted the Enhance_PHP_8_compatibility branch February 10, 2022 13:14
@konradoboza
Copy link

Hi @tedivm. Would you be able to publish a new tag (v0.16.1 AFAICS) containing this change? This issue kinda blocks our development so I was wondering if the fix can be released.

Thank you in advance. 😉

@tedivm
Copy link
Member

tedivm commented Feb 22, 2022

@konradoboza - sometime this week. If you check out the main branch I did a ton of work today migrating tests over to Github actions and merged some other PRs in as well, with the goal of speeding up development going forward. I'll be honest travis-ci shutting down open source projects happened at the worst time for me, as I was switching jobs and moving to Chicago. I didn't want to release until the tests were passing CI at a minimum, which is now happening in main.

@mlocati
Copy link
Contributor

mlocati commented Mar 10, 2022

sometime this week

It's been over a week now 😉

@tedivm
Copy link
Member

tedivm commented Mar 11, 2022

Yeah my cat died last weekend, kind of ruined my plans. Then I handed in notice at my job.

@tedivm
Copy link
Member

tedivm commented Mar 11, 2022

New release is up! 0.17.0, which will likely be the last line before v1.

@@ -209,7 +209,7 @@ public static function normalizeKeys($keys, $hash = 'md5')
* @throws Exception\RuntimeException
* @throws Exception\InvalidArgumentException
*/
public static function checkFileSystemPermissions($path = null, $permissions)
public static function checkFileSystemPermissions($path, string $permissions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't $permissions be an integer?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it should be.

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.

4 participants