Skip to content

Remove monolog dependency #785

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

Merged
merged 2 commits into from
Dec 1, 2023
Merged

Remove monolog dependency #785

merged 2 commits into from
Dec 1, 2023

Conversation

Melfo01
Copy link
Contributor

@Melfo01 Melfo01 commented Nov 29, 2023

Monolog repository is not used directly by this repository; so we can remove it ;)

@Melfo01 Melfo01 marked this pull request as ready for review November 29, 2023 16:06
@Melfo01
Copy link
Contributor Author

Melfo01 commented Nov 29, 2023

Hey @cleptric 👋,

My tests fail with Symfony 6.0. I think the problem is caused by symfony/psr-http-message-bridge package. In master, tests was launched with version 2.3.1, my PR used 6.4 version.

Do you know why and how to fix it?
Same question with PHPStan?

@ste93cry
Copy link
Contributor

I have nothing against merging these changes, but I fail to understand how it could help with your problem as monolog/monolog is a development requirement and does not affect at all third parties.

@Jean85
Copy link
Contributor

Jean85 commented Nov 30, 2023

Well, at the very least we're testing that it works fine with that major.

@ste93cry
Copy link
Contributor

Technically speaking, we don't even depend on monolog/monolog in this bundle, but rather on symfony/monolog-bundle, which is needed only for the E2E tests anyway. We could even remove the requirement on the lib imho, I don't see why it would be useful to keep it.

@Melfo01
Copy link
Contributor Author

Melfo01 commented Dec 1, 2023

I'm sorry, the issue wasn't caused by this repository.

However, I'm glad that it sparked a discussion on the necessity of having monolog repository in the dependencies.
I searched the code for dependencies with Monolog, and I didn't find anything either. If you'd like, I can remove this dependency in this pull request?

@ste93cry
Copy link
Contributor

ste93cry commented Dec 1, 2023

Yes please, that would be cool!

@Melfo01 Melfo01 changed the title Allow to use monolog version 3.0 Remove monolog dependency Dec 1, 2023
@ste93cry ste93cry merged commit bbbe69f into getsentry:master Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants