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 for latest symfony/cache-contracts #587

Closed
wants to merge 1 commit into from

Conversation

tacman
Copy link

@tacman tacman commented Dec 26, 2021

No description provided.

Copy link
Collaborator

@Jean85 Jean85 left a comment

Choose a reason for hiding this comment

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

Thank you for this PR but, as proven by the CI, we cannot merge this as is: https://github.com/getsentry/sentry-symfony/runs/4645506266?check_suite_focus=true#step:8:19

There's no way to have a piece of code compatible with both PSR-6 / Symfony Cache contracts 1 and 3 at the same time. And since the offending class (TraceableCacheAdapter) is already juggling between v1 and v2, I don't think there's an easy way to change it for now.

@quentint
Copy link

Hi and thanks for your answer.

I understand there's an incompatibility here, but most Symfony 6 apps will rely on symfony/cache and so on symfony/cache-contracts, both in recent versions; therefore not being able to install sentry/sentry-symfony.

How could we handle that?

@quentint
Copy link

Of course, current solution is

composer require sentry/sentry-symfony symfony/cache-contracts:^2.4

@Jean85
Copy link
Collaborator

Jean85 commented Jan 2, 2022

Exactly. The ecosystem is just getting ready for the 3.0 version, since it depends on psr/cache 3.0, which is another release that is available, but not widely supported.

To support 3.0 we would need to drop support for 1.0, which is still the mainly used one, by a long shot. Symfony itself supports ^1.0 | ^2.0 up until 5.4, which is the latest LTS, just fresh of release two months ago.

Sorry but I can't merge this as it is; if you can come up with a way to make the bundle compatible with 1 AND 3 at the same time, I will revisit this.

@ste93cry
Copy link
Collaborator

ste93cry commented Jan 2, 2022

Just to let you know, I'm working on this and I hope to share some news soon

@ste93cry
Copy link
Collaborator

ste93cry commented Jan 3, 2022

I'm closing this because it has been superseded by #588

@ste93cry ste93cry closed this Jan 3, 2022
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.

None yet

4 participants