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

Allow passing PSR-3 loggers to setLogger as they are compatible #1304

Merged
merged 4 commits into from
Aug 15, 2022

Conversation

Seldaek
Copy link
Contributor

@Seldaek Seldaek commented Jun 14, 2022

IMO you should really just use PSR-3 and require psr/log as it is anyway part of almost every PHP project at this point.. but at least this allows setting things up without getting static analysis warnings.

@CLAassistant
Copy link

CLAassistant commented Jun 14, 2022

CLA assistant check
All committers have signed the CLA.

@dcr-stripe
Copy link
Contributor

Thank you for the contribution @Seldaek and sorry about the delay! I'll discuss this with the team and get back to you but I think this makes a ton of sense!

@dcr-stripe
Copy link
Contributor

Hi @Seldaek - this looks good to me! I've brought your branch up to head - there's a small linter error:


     /**
-     * @param Util\LoggerInterface|\Psr\Log\LoggerInterface $logger the logger to which the library
+     * @param \Psr\Log\LoggerInterface|Util\LoggerInterface $logger the logger to which the library
      *   will produce messages
      */

Do you mind applying this change, and then I can give this an approval and merge?

Thank you for the contribution!

@Seldaek
Copy link
Contributor Author

Seldaek commented Aug 11, 2022

Sure, done.

@dcr-stripe dcr-stripe merged commit c36f057 into stripe:master Aug 15, 2022
Copy link
Contributor

@dcr-stripe dcr-stripe left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution @Seldaek , it'll go out with the next release!

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.

3 participants