New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom product flags using module #11614

Merged
merged 2 commits into from Dec 12, 2018

Conversation

Projects
None yet
7 participants
@kpodemski
Copy link
Contributor

kpodemski commented Dec 5, 2018

Questions Answers
Branch? develop
Description? title says it all
Type? improvement
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? install this example module: https://www.dropbox.com/s/l0st1g41aerrt4c/kpflagsmodifier.zip?dl=0

This change is Reviewable

@mickaelandrieu
Copy link
Contributor

mickaelandrieu left a comment

1 question

@@ -466,6 +467,11 @@ public function getFlags()
);
}
Hook::exec('productFlagsModifier', array(

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Dec 5, 2018

Contributor

is the name of the hook consistent with the others?

This comment has been minimized.

@kpodemski

kpodemski Dec 5, 2018

Contributor

i was wondering how to name this hook, we could have:
actionProductFlagsModifier

but it's not really any action

displayProductFlagsModifier

is more likely using to display something, not alter

so i decided to do this without prefix as some others hooks with same purpose, it should be it or action prefix, i am open for suggestions here

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Dec 6, 2018

Member

As far as I can tell, *Modifier hooks start with action. :)

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Dec 10, 2018

Contributor

Let's go for action prefix, thanks @Quetzacoalt91 !

This comment has been minimized.

@kpodemski

kpodemski Dec 10, 2018

Contributor

@mickaelandrieu done, do i need to squash those two commits or it is no longer needed in current workflow?

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Dec 12, 2018

All good, thank you @kpodemski

capture d ecran_753

@marionf marionf added QA ✔️ and removed waiting for QA labels Dec 12, 2018

@PierreRambaud PierreRambaud added this to the 1.7.6.0 milestone Dec 12, 2018

@PierreRambaud PierreRambaud merged commit 31e2ecb into PrestaShop:develop Dec 12, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@PierreRambaud

This comment has been minimized.

Copy link
Contributor

PierreRambaud commented Dec 12, 2018

Thanks @kpodemski and @marionf :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment