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

Move Symfony integration from Payum/Core into Payum/PayumBundle #838

Open
Tetragramat opened this issue Feb 13, 2020 · 2 comments · May be fixed by #995
Open

Move Symfony integration from Payum/Core into Payum/PayumBundle #838

Tetragramat opened this issue Feb 13, 2020 · 2 comments · May be fixed by #995
Assignees
Milestone

Comments

@Tetragramat
Copy link
Contributor

Payum/Core should not provide symfony integration when there is Payum/PayumBundle that does the same thing, but both do only part of the symfony integration.

Remove symfony integration from Payum/Core and move it into Payum/PayumBundle.

@Tetragramat
Copy link
Contributor Author

Thinking of making PR for this in near future but first I want to know if it would be accepted. @pierredup

I would copy contents of https://github.com/Payum/Payum/tree/master/src/Payum/Core/Bridge/Symfony into root of https://github.com/Payum/PayumBundle fixed namespaces and references inside of PayumBundle. Next would be tests from https://github.com/Payum/Payum/tree/master/src/Payum/Core/Tests/Bridge/Symfony.
That would be all needed to do in PayumBundle PR.

What I'm not completely sure what to do in Payum/Core to keep it BC.
Maybe replacing contents of all classes with extend to PayumBundle's version?

Is there better solution than this?

<?php
// src/Payum/Core/Bridge/Symfony/ReplyToSymfonyResponseConverter.php

namespace Payum\Core\Bridge\Symfony;

if (class_exists('Payum\Bundle\PayumBundle\ReplyToSymfonyResponseConverter')) {
  class ReplyToSymfonyResponseConverter extends Payum\Bundle\PayumBundle\ReplyToSymfonyResponseConverter {}
} else {
  class ReplyToSymfonyResponseConverter {
    // original content of the class
  }
}

@pierredup
Copy link
Member

@Tetragramat I think in payum/core, the classes can just be deprecated with a message specifying to use the classes in the bundle instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants