Extracted AppWebhook #1
Conversation
stereomon
force-pushed
the
feature/extracted-app-webhook
branch
from
41647e1 to
572710f
Compare
stereomon
force-pushed
the
feature/extracted-app-webhook
branch
from
ef36289 to
6f52357
Compare
src/Spryker/Glue/AppWebhookBackendApi/AppWebhookBackendApiConfig.php
Outdated
Show resolved
Hide resolved
| $container->set(static::FACADE_APP_WEBHOOK, static function (Container $container): AppWebhookBackendApiToAppWebhookFacadeInterface { | ||
| // The AppWebhookFacade will always be mocked | ||
| // @codeCoverageIgnoreStart | ||
| return new AppWebhookBackendApiToAppWebhookFacadeBridge($container->getLocator()->appWebhook()->facade()); |
There was a problem hiding this comment.
this is one repository, so the bridge is overkill here, just use facade via locator.
There was a problem hiding this comment.
No, let's keep our rules here as much as we can. This is not blocking anything. Don't want to spent time on this in this high pressure phase.
There was a problem hiding this comment.
we don't have rules for non-SCOS packages.
that is why I would not make it complex from the beginning.
Sure, you can ignore all my comments and just release it "as is", but remember that you always won't have time for refactoring later :)
There was a problem hiding this comment.
I don't ignore all your comments. I also don't have time to refactor to perfection now. Keeping this as in other Spryker module even if it's a package doesn't hurt IMO.
There was a problem hiding this comment.
BTW one sniffer complained about it that's why I added them everywhere. I don't like the bridges at all but for DevEx it's better to use one style if possible.
There was a problem hiding this comment.
My opinion about bridges is different, they make DevEx worse (more boilerplate code, more classes without a value)
There was a problem hiding this comment.
the sniffer is from SCOS, this package is another type of packages, so you should not use all SCOS sniffers and rules here.
PR Description
TBD
Checklist