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.

Sign up for GitHub

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

Jump to bottom

Fix some php 8 warnings #26494

Merged
merged 3 commits into from
Jun 7, 2021
Merged

Fix some php 8 warnings #26494

merged 3 commits into from
Jun 7, 2021

Conversation

rigrig
Copy link
Contributor

@rigrig rigrig commented Apr 10, 2021

Fixes PHP 8 warnings mentioned in #25806:

  • Error: Required parameter $internalPath follows optional parameter $mountId at /path/to/cloud/lib/private/Files/Config/CachedMountFileInfo.php#34
  • Error: Required parameter $tagManager follows optional parameter $fileIdentifier at /path/to/cloud/apps/files/lib/Helper.php#220
  • Error: ob_end_clean(): Failed to delete buffer. No buffer to delete at /path/to/cloud/lib/private/Files/View.php#449
  • Error: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero at /path/to/cloud/lib/public/AppFramework/Http/Template/PublicTemplateResponse.php#101

This only fixes the PHP 8 warnings, not deprecation warnings by 3rdparty code.

@solracsf solracsf added the 3. to review Waiting for reviews label Apr 10, 2021
This was referenced May 29, 2021
Closed
Closed
rigrig added 3 commits May 29, 2021 14:14
@rigrig
Fix functions taking optional parameters before required ones
f23d057 
PHP 8 shows deprecation warnings about this, see #25806
Removes the "default" values, as they actually are required parameters anyway.

Signed-off-by: Richard de Boer <[email protected]>
@rigrig
Fix a usort comparison function returning a boolean instead of an int…
a0d265b 
…eger

PHP 8 shows deprecation warnings about this, see #25806

Signed-off-by: Richard de Boer <[email protected]>
@rigrig
Check whether output buffering is active before turning it off
7990f95 
Before we just turned it off and @Suppressed the error if ob was not active.
In PHP 8 this error is no longer suppressed, so try to not cause it at all.

Signed-off-by: Richard de Boer <[email protected]>
@rigrig
Copy link
Contributor Author

rigrig commented May 29, 2021

Rebased onto master. (lib/private/Updater.php no longer needs fixing)

@ghost
Copy link

ghost commented Jun 7, 2021 •
edited by ghost
Loading

@MorrisJobke @kesselb @acsfer Review Please :)

@ghost
Copy link

ghost commented Jun 7, 2021

@rigrig is this already part of the list

[PHP] Error: Required parameter $mode follows optional parameter $a at /var/www/nextcloud/core/Controller/PreviewController.php#155

@rigrig
Copy link
Contributor Author

rigrig commented Jun 7, 2021

@rigrig is this already part of the list

[PHP] Error: Required parameter $mode follows optional parameter $a at /var/www/nextcloud/core/Controller/PreviewController.php#155

Yes: https://github.com/nextcloud/server/pull/26494/files#diff-5b14b6adeeb46970e6a2a91d54d0c4f2e41a36eabe8d6287bd3b1bfd94b89977L159-R160

I guess I didn't mention it because it wasn't listed in the issue #25806.

MorrisJobke
MorrisJobke approved these changes Jun 7, 2021
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code makes sense and cleans it nicely up 👍

@MorrisJobke MorrisJobke added this to the Nextcloud 22 milestone Jun 7, 2021
@MorrisJobke MorrisJobke merged commit 2ae60b4 into nextcloud:master Jun 7, 2021
@welcome
Copy link

welcome bot commented Jun 7, 2021

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@MorrisJobke
Copy link
Member

/backport to stable21

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jun 7, 2021
Merged
@ghost
Copy link

ghost commented Jun 7, 2021

/backport to stable21

Thank you!

@MorrisJobke MorrisJobke mentioned this pull request Jun 10, 2021
Merged
59 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jvoisin jvoisin jvoisin approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@blizzz blizzz Awaiting requested review from blizzz

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 22
Development

Successfully merging this pull request may close these issues.
5 participants
@rigrig @MorrisJobke @jvoisin @juliusknorr @solracsf