bobbrodie
commented
9 months ago Also there are a few other things that I've noticed in this
BitPayIpnProcessclass which I think can use some improvement. If you guys wouldn't mind I would really love to take this branch and make some additional changes that I think would be helpful. It is kinda little nitpicky stuff such as it is better to call$order->update_status()and pass a second note argument instead of calling$order->add_order_note()followed by$order->update_status(). I didn't want to mention all those things I found here in this PR, but if you guys are cool I can submit a PR against this branch with my suggestions. I can't do it until Monday since I'm taking off for a long weekend right now though.
This sounds great @shaunek-hero. Just one thing -- let's finish this PR first and merge it into master, then make other PRs. We don't release until tagged so there's no need to branch off this branch.
The reason I mention this is that this is going to be a part of two versions:
- 4.5.0: Only adding new capabilities / bug fixes
- 5.0.0 (will be a new 5.0.x branch): We can introduce backward incompatibilities if necessary -- note: we agree that we'd like to refactor as well and are working on some of that as we roll out new major versions.
One thing that I'll share here that's coming up is that we are working on adding automated tests to this plugin as well. It didn't have them in the past and would help with a lot of this. We're also planning on having those tests run against a matrix of versions of PHP and WooCommerce. The other projects, like the SDKs have unit and functional test suites, and while a lot of WP plugins don't, we want to introduce some testing to help catch the important parts of the flow.
Thanks!
Also there are a few other things that I've noticed in this
BitPayIpnProcessclass which I think can use some improvement. If you guys wouldn't mind I would really love to take this branch and make some additional changes that I think would be helpful. It is kinda little nitpicky stuff such as it is better to call$order->update_status()and pass a second note argument instead of calling$order->add_order_note()followed by$order->update_status(). I didn't want to mention all those things I found here in this PR, but if you guys are cool I can submit a PR against this branch with my suggestions. I can't do it until Monday since I'm taking off for a long weekend right now though.