Closed codingconcepts closed 6 years ago
Thanks a lot! LGTM!
I'll write to Adyen support to activate checkout flow on a test account, so we can re-enable tests
@codingconcepts this commit https://github.com/zhutik/adyen-api-go/commit/508699a3d125d4acbd05a75564beade5eb9be10a totally make sense to me. Please create a separate PR and I will merge it.
In this branch, there is extra commit with defer
and ignoring error for Close
, but I guess the issue is solved already, so we don't need it anymore. What do you think?
Yeah, we can ignore that. Your change to defer and log on error does what we need.
On the subject of logging on error... Would you consider moving logging to something like logrus? I only ask because the library currently logs a LOT of stuff (and some potentially sensitive stuff too, given that all requests and responses are logged).
I think we should either have two loggers, one for stdout and one for stderr, or using levelled logging so that these trace-level logs can be ignored.
I'll be honest, when I spin up an instance of the Adyen client, I pass in ioutil.Discard
as the io.Writer
as I find it too verbose :P (and potentially to unsafe, which is a more important reason of course!)
Cheers mate!
Rob
Hey Igor,
Here's another branch for you :)
This one adds some interactivity with the CheckoutAPI (namely a method that we need to list the available payment methods etc.)
I've marked the test as skipped at the moment as I don't think you'll have access to the CheckoutAPI at the moment.
Would you also consider merging my
feature/client-reuse
branch please? Currently in the execute methods of adyen.go, we're creating a new client for every request. This goes against the best practices outlined in the stdlib documentation for http.Client.Cheers!
Rob