-
Notifications
You must be signed in to change notification settings - Fork 96
Add displayCartExtraProductInfo hook #699
New iss 8000 ue
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
Add displayCartExtraProductInfo hook #699
Conversation
Questions | Answers |
---|---|
Description? | Add displayCartExtraProductInfo hook |
Type? | new feature |
BC breaks? | no |
Deprecations? | no |
Fixed ticket? | Fixes PrestaShop/PrestaShop#38692 |
Sponsor company | Creabilis.com |
How to test? | See issue |
PR merged, well done! Message to @PrestaShop/committers: do not forget to milestone it before the merge. |
No QA here? 🤔 |
@nicosomb Not needed, it's only a smarty hook and the syntax is identical to all other simmilar hooks. I verified it on a live store. |
@nicosomb But thanks for keeping an eye on things, as always. |
Sorry, @Hlavtox but even though we agree there's little risk in this modification it's not up to you to unilaterally decide that QA is not needed and ignore this step by using your privileges to merge... Remember, with great power comes great responsibility 😅 Just a kind warning: if you keep abusing those privileges, I'm afraid we'll be forced to revoke your merge authorisations |
@jolelievre Jo, this topic again. :-) I’ve seen much more impactful PRs merged without QA and no warnings were issued. If we’re going to strictly enforce rules, I’m all for it — but it should be done consistently, not selectively. |
@Hlavtox @jolelievre @nicosomb WDYT guys? |
@ShaiMagal @Hlavtox One simple solution that has been followed in the past is
At the end of the day QA analysts are the ones deciding whether or not they should test it 😉 |
Yep, to put it simply we just need to ask the QA if its ok to merge it |
Merge pull request PrestaShop#699 from jf-viguier/hookDisplayCartExtraProductInfo