Port to use ominpay/common:^3 - Githubissues

instride-ch / omnipay-datatrans

Omnipay Datatrans

MIT License

1 stars 8 forks source link

Port to use ominpay/common:^3 #1

Open dpfaffenbauer opened 7 years ago

judgej commented 6 years ago

I may be able to do this for you as a part of another project. There are still a few details to be worked out on Omnipay 3.0, but just branching this package at the right point is the important first step.

I also need PayPal and E-Finance support (for both 2.x and 3.x). Is that something that would be useful to you?

dpfaffenbauer commented 6 years ago

Totally! Please go ahead :) 👍

dpfaffenbauer commented 6 years ago

Actually porting it to omnipay 3 isn't that big deal at all, I did worldpay a few weeks back: https://github.com/thephpleague/omnipay-worldpay/pull/29

judgej commented 6 years ago

Yes, with most gateways it should be fairly simple. The biggest problem is managing two branches, because at the point you fork off 3.0, any additions or fixes will likely need to be done to both branches. Until you stop supporting 2.x, that is. We do need that critical mass to reach that point though. Hopefully soon the momentum will grow (it's kind of waiting on PSR-18 to be agreed so it can be detached from Guzzle as a dependency).

dpfaffenbauer commented 6 years ago

Yes thats right, but for datatrans that shouldn't be too hard. I mean, there weren't that much changes since I started it.

judgej commented 6 years ago

Cool. Anyway, I've been tasked with extending this driver (at some point soon), and mainly wanted to check with you whether to submit PRs or fork it under a different vendor name for speed. It makes sense not fragmenting packages with multi-vendor versions, so if you are here, happy in principle to accept PRs for extending the payment methods, then I'm happy to work on your driver.

dpfaffenbauer commented 6 years ago

I'm down :)

judgej commented 6 years ago

Just FYI, I have changed the fork of this project quite a bit, and it is probably going to be difficult to merge it in without treating it as a complete replacement. Some concepts I've changed:

The first thing I do with thew XML responses is to flatten them into flat arrays. These arrays can then be handled in the same way as the non-XML responses. XML is okay for navigating data, but not so easy to test, and SimpleXML is still a bit buggy.
The response data can be handled as GET, POST, XML in header or XML in body.
All signing options are supported: sign1, sign2, and the HMAC key1 and key2.
I'm trying not to distinguish between the XML and non-XML parts of the API. The merchant site does not care of the transport format, so I'm aiming to abstract that completely in the driver.
I've increased the minimum PHP version to 5.6 and used traits and 5.6 syntax.

All throughout, I've got the 3.x conversion in mind, so have tried to abstract messaging where I can so Guzzle3+Symfony can be switched over to PSR-7+PSR-15+PSR-18 in just one or two places.

dpfaffenbauer commented 6 years ago

Alright, whatever you think is ok. Since it's gonna be a BC break anyway, its fine.

Regarding PHP Version, don't need to require PHP5.6 at min version, omnipay/omnipay:3.0.x-dev already does that. But, I just saw that omnipay/common requires at least php 7.1 and omnipay/omnipay requires omnipay/common. :D, So, I guess the min is PHP 7.1 then.