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

Add parameter to setMedia #8588

Merged
merged 1 commit into from Dec 11, 2017

Conversation

Projects
None yet
6 participants
@aw73
Contributor

aw73 commented Dec 6, 2017

if not Fatal Error with PHP 7.2.0 :
PHP Fatal error: Declaration of AdminDashboardControllerCore::setMedia()
must be compatible with AdminControllerCore::setMedia($isNewTheme = false)
in /prestashop/controllers/admin/AdminDashboardController.php on line 539

Questions Answers
Branch? develop
Description? Fatal error with PHP 7.2.0 on back-office request
Type? bug fix
Category? BO
BC breaks? ?
Deprecations? ?
Fixed ticket?
How to test? Install the last version of PHP 7.2.0 (5/12/2017)

Important guidelines


This change is Reviewable

Add parameter to setMedia
if not Fatal Error with PHP 7.2.0 :
PHP Fatal error:  Declaration of AdminDashboardControllerCore::setMedia() 
must be compatible with AdminControllerCore::setMedia($isNewTheme = false) 
in /prestashop/controllers/admin/AdminDashboardController.php on line 539
@prestonBot

This comment has been minimized.

Show comment
Hide comment
@prestonBot

prestonBot Dec 6, 2017

Collaborator

Hello aw73!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

Collaborator

prestonBot commented Dec 6, 2017

Hello aw73!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Dec 11, 2017

Contributor

Hi @aw73,

thanks for your patch :)

Mickaël

Contributor

mickaelandrieu commented Dec 11, 2017

Hi @aw73,

thanks for your patch :)

Mickaël

@mickaelandrieu mickaelandrieu merged commit 3393087 into PrestaShop:develop Dec 11, 2017

2 checks passed

Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@SharakPL

This comment has been minimized.

Show comment
Hide comment
@SharakPL

SharakPL Jan 22, 2018

It's not the only one problem of this type. Current release (1.7.2.4 dated Nov 2017) has many problems here. After installation with php 7.2 and deleting '/install' folder, admin isn't renamed with adminXXXXXXXX and BO doesn't load at all.

FatalErrorException in AdminDashboardController.php line 0: Compile Error: Declaration of AdminDashboardControllerCore::setMedia() must be compatible with AdminControllerCore::setMedia($isNewTheme = false)

Similar 2 errors are in AdminLoginController.php with viewAccess() and again setMedia().
There's a lot of these errors in both BO and FO mainly because of missing arguments in functions. But I guess the problem is that functions are declared twice in different classess used and those declarations differ in arguments (sloppy coding :p ).
Also many warnings and errors of depreciated functions.

I've just tried prestashop_1.7.3.0-beta.1 and nothing of this was fixed yet :(

It's even worse for prestashop 1.6.1.17 on php7.2 server. Missing Mcrypt warning at the start and then installation stopes with error after filling 3 ps_shop*tables.

I guess it's no go for prestashop on php7 at this time :(

SharakPL commented Jan 22, 2018

It's not the only one problem of this type. Current release (1.7.2.4 dated Nov 2017) has many problems here. After installation with php 7.2 and deleting '/install' folder, admin isn't renamed with adminXXXXXXXX and BO doesn't load at all.

FatalErrorException in AdminDashboardController.php line 0: Compile Error: Declaration of AdminDashboardControllerCore::setMedia() must be compatible with AdminControllerCore::setMedia($isNewTheme = false)

Similar 2 errors are in AdminLoginController.php with viewAccess() and again setMedia().
There's a lot of these errors in both BO and FO mainly because of missing arguments in functions. But I guess the problem is that functions are declared twice in different classess used and those declarations differ in arguments (sloppy coding :p ).
Also many warnings and errors of depreciated functions.

I've just tried prestashop_1.7.3.0-beta.1 and nothing of this was fixed yet :(

It's even worse for prestashop 1.6.1.17 on php7.2 server. Missing Mcrypt warning at the start and then installation stopes with error after filling 3 ps_shop*tables.

I guess it's no go for prestashop on php7 at this time :(

@Quetzacoalt91

This comment has been minimized.

Show comment
Hide comment
@Quetzacoalt91

Quetzacoalt91 Jan 22, 2018

Member

Be careful, this may be true for PHP 7.2, but 7.0 & 7.1 are working properly.
PHP 7.2 compatibility is in progress for PrestaShop 1.7.4 with #8246

Member

Quetzacoalt91 commented Jan 22, 2018

Be careful, this may be true for PHP 7.2, but 7.0 & 7.1 are working properly.
PHP 7.2 compatibility is in progress for PrestaShop 1.7.4 with #8246

@SharakPL

This comment has been minimized.

Show comment
Hide comment
@SharakPL

SharakPL Jan 23, 2018

@Quetzacoalt91 Good to know. I must have missed that PR, sorry. I've waited for 100% stable 1.7 to finally switch from 1.6.1.17 and expected of it to be stable on php7. I guess it's more of php7 issue as it's still being developed. By saying 'no go for prestashop on php7' I meant basiacally 'no go for php7 yet' as I don't want to use unstable php either.
Thank you for all your hard work.

SharakPL commented Jan 23, 2018

@Quetzacoalt91 Good to know. I must have missed that PR, sorry. I've waited for 100% stable 1.7 to finally switch from 1.6.1.17 and expected of it to be stable on php7. I guess it's more of php7 issue as it's still being developed. By saying 'no go for prestashop on php7' I meant basiacally 'no go for php7 yet' as I don't want to use unstable php either.
Thank you for all your hard work.

@eternoendless eternoendless added this to the 1.7.4.0 milestone Apr 13, 2018

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