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

Fix compatibility with PHP 5.6 for PS exception #10865

Merged
merged 2 commits into from Oct 8, 2018

Conversation

Projects
None yet
4 participants
@sarjon
Member

sarjon commented Oct 4, 2018

Questions Answers
Branch? 1.7.5.x
Description? Throwable was introduced in PHP 7, PR removes it.
Type? bug fix
Category? CO
BC breaks? no
Deprecations? no
Fixed ticket? n/a
How to test? See code changes.

This change is Reviewable

@sarjon

This comment has been minimized.

Member

sarjon commented Oct 4, 2018

i dont really like this exception. :/ i'd prefer having CoreException and maybe TranslationAwareException extends CoreException, wdyt? ping @jolelievre @matks

@matks

This comment has been minimized.

Contributor

matks commented Oct 5, 2018

@sarjon You mean renaming PrestaShopCoreException into CoreException ?

I'm fine with it, both names are OK.

@sarjon

This comment has been minimized.

Member

sarjon commented Oct 5, 2018

@matks not really, i dont like that PrestaShopCoreException exception is translation aware (with message & domain required), i prefer that exceptions wouldnt be like that, wdyt?

@matks

This comment has been minimized.

Contributor

matks commented Oct 5, 2018

@sarjon This is a nice improvement 👍
I would suggest TranslationAwareException => TranslationAwareCoreException to keep the "Core" inside the name
And since we extend the main one we should be able to introduce it / use it without BC break

@sarjon

This comment has been minimized.

Member

sarjon commented Oct 5, 2018

@matks i had very similar idea. :)

@matks matks merged commit 3fe2ca0 into PrestaShop:1.7.5.x Oct 8, 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

@sarjon sarjon deleted the sarjon:fix-ps-core-exception branch Oct 8, 2018

@matks matks added this to the 1.7.5.0 milestone Oct 9, 2018

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