pimcore / ecommerce-framework-bundle

Ecommerce Framework community bundle provides e-commerce functionality such as product listing and filtering, pricing, carts and checkouts for Pimcore.
https://pimcore.com/docs/platform/Ecommerce_Framework/
Other
11 stars 33 forks source link

[Task]: Fix logger entry in OrderAgent #31

Closed kingjia90 closed 1 year ago

kingjia90 commented 1 year ago

Followup https://github.com/pimcore/ecommerce-framework-bundle/pull/25

See also https://github.com/pimcore/ecommerce-framework-bundle/pull/25#discussion_r1164209637 for details

kingjia90 commented 1 year ago

LGTM, it was also a problem with phpstan so I fixed it there too. I fixed it a little different though, https://github.com/pimcore/ecommerce-framework-bundle/pull/37/files#diff-7febe5b4c99069abacef16bf82f8bda7a00cbaea701a69e023f09e17182a6734R529 Wdyt?

Look also good to me, as far as i saw, we do not have any particular rule to stick with and be consistent to, so there are no hard rules yet.

One potential approach would be $this->logger->info('Update payment called with status {status}.', array('status' => $status));, as seen on https://github.com/Seldaek/monolog/issues/249#issuecomment-26150242, to avoid the use of print_r and have a more fixed message part and a variable one (array/context), but we might have to change all the usages.

Closing this PR in favor of #37