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

Use _PARENT_THEME_NAME_ constant for consistency #6709

Merged
merged 1 commit into from Oct 18, 2016

Conversation

Projects
None yet
2 participants
@hubiktomas
Contributor

hubiktomas commented Oct 18, 2016

Undefined constant _PARENT_THEME_NAME_ - the correct constant in the if should be _PS_PARENT_THEME_DIR_ as used in the rest of the file.

Questions Answers
Branch? develop
Description? Undefined constant _PARENT_THEME_NAME_ - the correct constant in the if should be _PS_PARENT_THEME_DIR_ as used in the rest of the file.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? The warnings in webserver log should be gone. The issue was probably breaking some plugins/templates when using more themes (not sure with that).
Undefined constant _PARENT_THEME_NAME_
The correct constant in the if should be _PS_PARENT_THEME_DIR_ as used in the rest of the file.
@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Oct 18, 2016

Contributor

Hi,

What version are you testing? I can't reproduce the issue. Especially if _PARENT_THEME_NAME_ is not defined, _PS_PARENT_THEME_DIR_ shouldnt be defined either.

Contributor

julienbourdeau commented Oct 18, 2016

Hi,

What version are you testing? I can't reproduce the issue. Especially if _PARENT_THEME_NAME_ is not defined, _PS_PARENT_THEME_DIR_ shouldnt be defined either.

@julienbourdeau julienbourdeau self-assigned this Oct 18, 2016

@hubiktomas

This comment has been minimized.

Show comment
Hide comment
@hubiktomas

hubiktomas Oct 18, 2016

Contributor

Hi,

you are right. It was during the installation process so I just might have some files missing after upload using FTP. I deleted everything and tried it once again and no warning occurred this time. Sorry for false alarm. :)

However I still think that changing the constant is a good idea as you really would like to test whether the one that you are using is defined, not the one in the if. You are right that there two constants are connected and if one is defined, the other should be too, but the code is a little bit confusing at this place.

Contributor

hubiktomas commented Oct 18, 2016

Hi,

you are right. It was during the installation process so I just might have some files missing after upload using FTP. I deleted everything and tried it once again and no warning occurred this time. Sorry for false alarm. :)

However I still think that changing the constant is a good idea as you really would like to test whether the one that you are using is defined, not the one in the if. You are right that there two constants are connected and if one is defined, the other should be too, but the code is a little bit confusing at this place.

@julienbourdeau julienbourdeau changed the title from CO: Undefined constant _PARENT_THEME_NAME_ fix to Use _PARENT_THEME_NAME_ constant for consistency Oct 18, 2016

@julienbourdeau

This comment has been minimized.

Show comment
Hide comment
@julienbourdeau

julienbourdeau Oct 18, 2016

Contributor

Yeah it's more readable to test on the constant we use after.
Thanks for your contribution!

Contributor

julienbourdeau commented Oct 18, 2016

Yeah it's more readable to test on the constant we use after.
Thanks for your contribution!

@julienbourdeau julienbourdeau merged commit a802f99 into PrestaShop:develop Oct 18, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment