Upgrade from 3.0.0 to 3.1.0 issue with psr/log requirement

[tipo94]

Upgrading from 3.0.0 to 3.1.0 has for consequence to upgrade psr/log from 1..4 to 3.0.0 which throw an error as the setLogger method is not compatible with the upgrade version of the LoggerAwareInterface:

Declaration of Mautic\Api\Api::setLogger(Psr\Log\LoggerInterface $logger) must be compatible with Psr\Log\LoggerAwareInterface::setLogger(Psr\Log\LoggerInterface $logger): void

[escopecz]

That's bad. How would you suggest to solve it? I can see 2 options:

1. Rebrand the release as major release with breaking changes - 4.0.0
2. Downgrade the psr/log back (https://github.com/mautic/api-library/issues/268)

cc @jonnitto @dakur

[dakur]

As for semver, there should be 3.1.1 release which reverts the upgrade to newer psr/log, and then 4.0.0 release containing the upgraded psr/log. If this is not a problem for you, I think it's the best option. :-)

[dakur]

@escopecz What do you think?

[escopecz]

@dakur yes, great idea. Would you have time to prepare the revert of psr/log in a PR? I'll try to then make the 2 releases.

[dakur]

I can have a look at it, but not earlier than in two weeks, sorry.

[timothyoesch]

I am still getting this error when calling the newApi method on a correctly instantiated Auth object:

PHP Fatal error: Declaration of Mautic\Api\Api::setLogger(Psr\Log\LoggerInterface $logger) must be compatible with Psr\Log\LoggerAwareInterface::setLogger(Psr\Log\LoggerInterface $logger): void

Any updates?

[escopecz]

@timothyoesch please test and comment on https://github.com/mautic/api-library/pull/279 to help getting it to a mergeable state.

[Rocksheep]

I have made a pull request to fix this issue. An update of the Psr\Log package was not required as the setLogger method in the API class never adhered to the interface according to the docblocks of the interface. I have corrected this mistake and now it should work again.

[christian-beckmann]

To use the fork, just add this to your composer.json

{
"repositories": [ { "type": "vcs", "url": "https://github.com/Rocksheep/api-library.git" } ], 
"require": { "mautic/api-library": "dev-upgrade-to-php8", }
}

[Nicscott01]

Thank you, this fixed the problem for me, too.

When will this fix be available in the "official" repo?

[escopecz]

Please comment your test results on the pull request itself. There must be a record of people testing it before it can be merged.

[dakur]

I can't understand this. We are talking about upgrading to new php version, not a complex new feature. What do you need to test about it from number of people? It just works or it doesn't, you can try it manually or have a test for it. PR author probably tested it, what do you need more?

Also, there were multiple PRs for this but because of the ridiculous requirement of signing CLA which some of contributors have problem with, it discourages and demotivates from contributing (at least me). Further, I give you an explicit approval to reuse the code I wrote (few lines) in a new PR, but you (or someone) didn't put any effort into it. That feels to me like you/mautic team doesn't care much about this library.

I just needed to express frustration/disappointment, sorry.

[dakur]

To be fair, I know there is plenty of work on mautic not just this lib. Just can't understand how it can be so difficult to fix/finish this for library maintainers. Anyway thank you for your work on whole mautic.

[RCheesley]

Hi @dakur I appreciate it is confusing if you are not involved with Mautic on a wider scale.

Here is our code governance: https://contribute.mautic.org/community-structure/governance/code-governance - the reason we require two people to test is because human beings miss things. More often than not, the second tester picks up things that were missed by the first tester.

The way GitHub works in Mautic is that PR merging is blocked until there is a review on the PR using the process documented here: https://contribute.mautic.org/contributing-to-mautic/tester#leaving-your-review which is why there was an ask to leave testing comments and reviews on the PR, not on the issue. It saves the core team having to go back and forth from issue to PR to find what has and hasn't been tested. This is standard practice across all Mautic repos.

As regards the contributors agreement, it is a mandatory requirement across all Mautic repositories, if people are not willing to sign it, we cannot accept their contributions. This is a black and white thing, no grey area.

At this time, the focus of our very small team of volunteers is getting the work done and tested for Symfony 5 support, so that is where the attention has to be focused right now. It's not a case of someone not caring about re-writing someone's code because they don't want to sign the agreement, it's a case of prioritising the limited time we have available for the things that the wider community requires in order to be using a secure version of the framework that underpins Mautic.

I expect that someone will get around to doing this at some point, but right now we just do not have the spare bandwidth to do so.

Ways you can help:

1. Join the Open Source Friday sprints every week in #t-product on Slack (get an invite at https://mau.tc/slack-invite) - test instructions are here and there is a pinned issue in mautic/mautic on GitHub each week with the areas of focus
2. Make the PR yourself, after signing the contributors agreement (it's a 2 minute job online with Docusign)
3. Help by testing the API library PRs as they are often overlooked due to our limited capacity - clone the repo, apply the PR, review and leave your feedback. See the Readme here: https://github.com/mautic/api-library#contributing