search

killbill / killbill-adyen-plugin

Kill Bill plugin for Adyen
https://killbill.io
Apache License 2.0
10 stars 31 forks source link

New Adyen changes #134

Closed reshmabidikar closed 1 year ago

reshmabidikar commented 1 year ago

I leave a review for issues that I had previously when working on this.

Thank you @xsalefter for checking this in so much detail 🙏 . I need a few clarifications for which I have added comments. Please take a look when you get a chance.

xsalefter commented 1 year ago

@reshmabidikar no worries, I just remembered that I changes things on this plugins and it is still on my local branch, so I just diff it and write it here. I write some answers for your questions. Let me know if you have more.

reshmabidikar commented 1 year ago

That being said, I don't think this is a blocker for the merge, this could be done in another PR

This @pierre. I think we should fix the tests to use TestNG instead of Junit (Since our ci/release workflows depend on various test groups). I will do that as part of a subsequent PR. For the other comments, I will create a task in a backlog if that is okay.

So this PR can be merged.

reshmabidikar commented 1 year ago

Filed https://github.com/killbill/technical-support/issues/88 for the comments related to cleaning up the code.