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

feat: merge symfony bridge (symfony 7) #560

Merged
merged 2 commits into from
Apr 11, 2024

Conversation

Chris53897
Copy link
Contributor

Based on #557

I did the following changes:

@Chris53897
Copy link
Contributor Author

I did a test in a real life project and payment worked got us.

add ContainerAwareInterface and ContainerAwareTrait to support symfony 7
@pierredup pierredup force-pushed the feature/merge-symfony-bridge branch from 6d7098a to 5d74dc6 Compare April 8, 2024 09:24
@pierredup
Copy link
Member

Thanks @Chris53897

fixed the namespace for the tests-files

I added this change to #557. It's better to have all related changes in the same PR

integrated the trait in interface in the repo

I wonder if it's even worth it to support the setContainer calls with a custom interface and trait. Since there are only 3 internal classes that uses this, it might be better to just inject the container into the constructor of the classes directly and get rid of the interface and trait completely.

@Chris53897
Copy link
Contributor Author

I am fine to get rid of them. I do not know how to implement it.
My main goal is to move towards symfony 7 support.
I archived this now (with a copy of this branch).

@pierredup
Copy link
Member

I have pushed a change to remove the ContainerAwareInterface and ContainerAwareTrait

@Chris53897
Copy link
Contributor Author

Thanks. Is there something needed from my side to get it merged?

@pierredup pierredup merged commit e26dd9a into Payum:master Apr 11, 2024
12 checks passed
@Chris53897 Chris53897 deleted the feature/merge-symfony-bridge branch April 11, 2024 13:51
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.

None yet

3 participants