Skip to content
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

Switch from deprecated 'header' hook to 'displayHeader' #126

Merged
merged 2 commits into from Dec 29, 2021

Conversation

tswfi
Copy link
Contributor

@tswfi tswfi commented Dec 29, 2021

Questions Answers
Description? 'header' hook is deprecated, use 'displayHeader' instead
Type? improvement
BC breaks? no
Deprecations? no
Fixed ticket? Fixes PrestaShop/PrestaShop#27129
How to test? Check that the module works and add to basket works after upgrading the module

Not sure if the version number should be increased in a PR like this or should there be another pr for it..

config.xml Outdated
@@ -2,7 +2,7 @@
<module>
<name>productcomments</name>
<displayName><![CDATA[Product Comments]]></displayName>
<version><![CDATA[5.0.1]]></version>
<version><![CDATA[5.0.2]]></version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<version><![CDATA[5.0.2]]></version>
<version><![CDATA[5.0.1]]></version>

@@ -45,7 +45,7 @@ public function __construct()
{
$this->name = 'productcomments';
$this->tab = 'front_office_features';
$this->version = '5.0.1';
$this->version = '5.0.2';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->version = '5.0.2';
$this->version = '5.0.1';

Copy link
Contributor

@PierreRambaud PierreRambaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version 5.0.1 hasn't been released yet

@PierreRambaud PierreRambaud added this to the 5.0.1 milestone Dec 29, 2021
@tswfi
Copy link
Contributor Author

tswfi commented Dec 29, 2021

The version 5.0.1 hasn't been released yet

Ah, so it seems. I can add the hook upgrade there.

@tswfi
Copy link
Contributor Author

tswfi commented Dec 29, 2021

btw, is there a best practice on registerhook / unregisterhook. which one should be done first?

@PierreRambaud
Copy link
Contributor

btw, is there a best practice on registerhook / unregisterhook. which one should be done first?

Nope, there is nothing related to it ^^

@PierreRambaud PierreRambaud merged commit d05fbcb into PrestaShop:dev Dec 29, 2021
@PierreRambaud
Copy link
Contributor

Thank you @tswfi

@PierreRambaud PierreRambaud changed the title switch from deprecated 'header' hook to 'displayHeader' Switch from deprecated 'header' hook to 'displayHeader' Dec 29, 2021
@tswfi tswfi deleted the 27129_fix_deprecated_hooks branch December 29, 2021 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants