Product now has condition attribute that is displayed on product detail page and is generated in google feed#7
Conversation
Miroslav-Stopka
left a comment
Miroslav-Stopka
left a comment
There was a problem hiding this comment.
Hi @TomasLudvik ,
thank you for your contribution. Im ready for review :)
|
|
||
| class GoogleFeedItemFactory extends BaseGoogleFeedItemFactory | ||
| { | ||
| public function __construct( |
There was a problem hiding this comment.
Is this constructor really needed in this class? I mean, there are no differences between parent constructor and this one.
There was a problem hiding this comment.
It is not, I have removed it
|
|
||
| Shopsys\ShopBundle\Model\: | ||
| resource: '../../Model/*/*{Facade,Factory}.php' | ||
| resource: '../../Model/**/*{Facade,Factory}.php' |
There was a problem hiding this comment.
Please, explain why two asterisks are better than one :)
There was a problem hiding this comment.
Two asterisks mean that the depth of nesting can be of unspecified level and it is because of Model/Product/Feed/Google/GoogleFeedItemFactory
| alias: Shopsys\ShopBundle\Model\Product\ProductDataFactory | ||
|
|
||
| Shopsys\ShopBundle\Model\Product\ProductDataFactory: ~ | ||
|
|
There was a problem hiding this comment.
Why did you register this services into services_test? They are already registered in services.yml I think. And services_test.yml is loaded after services.yml so I dont see the pros of this change. Can you explain it to me?
There was a problem hiding this comment.
Because all services registered in services.yml are private and I need this one in tests.
|
Review and tests are done :) Please answer my questions. Thank you. |
TomasLudvik
force-pushed
the
tl-feed-condition
branch
from
c2a54bf to
026ac43
Compare
2 participants
Uh oh!
There was an error while loading. Please reload this page.