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: replace logger #251

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vitormattos
Copy link

The previous logger approach was removed at Nextcloud 30 and was needed to be replaced by Psr\Log\LoggerInterface

This PR fix:

Error: Call to undefined method OC\Server::getLogger() in /var/www/html/apps-writable/cms_pico/lib/Migration/Version010000From000908.php:65

The previowus logger approach was removed at Nextcloud 30 and was needed
to be replaced by Psr\Log\LoggerInterface

Signed-off-by: Vitor Mattos <[email protected]>
@PhrozenByte PhrozenByte changed the base branch from master to cms_pico-1.0 October 21, 2024 17:43
@PhrozenByte PhrozenByte changed the base branch from cms_pico-1.0 to master October 21, 2024 17:43
@PhrozenByte
Copy link
Collaborator

PhrozenByte commented Oct 21, 2024

Thank you for your contribution! ❤️

First of all, please note that cms_pico is abandoned dev-wise, thus there isn't going to be a new release in the foreseeable future. There's currently no stable version of cms_pico supporting any of Nextcloud's still officially supported branches; people still using cms_pico do so at their own risk and use custom app builds (usually) based on the cms_pico-1.0 branch.

You can ignore the failing CI, just wanted to see whether it's more broken than last time I checked - it indeed is.

If you're willing to invest more time nevertheless, can you please rebase on the cms_pico-1.0 branch? The master branch is the dev version of cms_pico v2.0. Just switching the PR's base unfortunately doesn't work properly. Did you check all usages of $logger to be compatible with the new LoggerInterface? Unfortunately I don't have the time to do a proper review of your PR, but the changes look good; so, if you affirm that you did check all usages, I'll merge your PR nevertheless. Thanks again!

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.

2 participants