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

Fix name of dymanic var $mountOptions to fix PHP 8.2 compatibility #35400

Merged
merged 2 commits into from
Dec 5, 2022

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Nov 24, 2022

This fixes some tests for files_sharing on PHP 8.2.

As the var was unused I removed the line instead of adding the var as a class property.

Signed-off-by: Côme Chilliet come.chilliet@nextcloud.com

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc self-assigned this Nov 24, 2022
@come-nc come-nc requested review from PVince81, icewind1991, a team and skjnldsv and removed request for a team November 24, 2022 10:54
@come-nc come-nc added the 3. to review Waiting for reviews label Nov 24, 2022
@come-nc come-nc added this to the Nextcloud 26 milestone Nov 24, 2022
@come-nc come-nc mentioned this pull request Nov 24, 2022
19 tasks
@come-nc
Copy link
Contributor Author

come-nc commented Nov 24, 2022

See 2f03fca
(so maybe remove as well $options?)

@icewind1991
Copy link
Member

This is might be used through

$storage->setMountOptions($mount->getOptions());

so it's probably better to just add the property

@come-nc
Copy link
Contributor Author

come-nc commented Nov 28, 2022

This is might be used through

$storage->setMountOptions($mount->getOptions());

so it's probably better to just add the property

But the property would be private so who would read it?

From what I understand if ($storage->instanceOfStorage(Common::class)) { returns true because OCA\Files_Sharing\SharedStorage is a wrapper around an instance of Common?
It should either do nothing or set the options on the wrapped storage, no?

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Contributor Author

come-nc commented Dec 5, 2022

@icewind1991 I’ve put back the property and just fixed the naming so that we can merge this and move on.

@come-nc come-nc changed the title Remove dymanic var $mountOptions to fix PHP 8.2 compatibility Fix name of dymanic var $mountOptions to fix PHP 8.2 compatibility Dec 5, 2022
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

👍

@come-nc come-nc merged commit f95aa23 into master Dec 5, 2022
@come-nc come-nc deleted the fix/remove-unused-dynamic-var-in-files_sharing branch December 5, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants