Event dispatched along with retailer data when set.

Smile-SA / magento2-module-store-locator

Magento 2 store locator module

50 stars 57 forks source link

Event dispatched along with retailer data when set. #19

Closed Chacalbis closed 6 years ago

Chacalbis commented 6 years ago

This event will be used to save customer's favorite retailers.

TheGuit commented 6 years ago

@romainruaud @afoucret pouvez-vous regarder cette pull-request et nous faire un retour ?

Cordialement,

delyriand commented 6 years ago

Hi @Chacalbis and @TheGuit,

I have 2 problems with this PR:

indentation and spacing (the end parenthesis and the semicolon of the dispatch method should be at a new line, missing spacing around assignment operator in array)
why not put a after plugin on setRetailer method?

Chacalbis commented 6 years ago

Hi @delyriand,

Indeed, defining a plugin with an after method sounds like a better alternative. I guess such customizations should be done using plugins for public methods like setRetailer whenever possible. We just didn't give it a thought at the time.

We can either drop this PR and make the appropriate changes in our code. Or we can fix these indentation and spacing issues.

Thank you for your feedback

delyriand commented 6 years ago

Hello @Chacalbis,

if you can modify your code instead of an observer, i prefer this solution.

I closed the PR but do not hesitate to contribute again ;)