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

MAGECLOUD-1193: Enable strict errors #71

Merged
merged 3 commits into from
Oct 31, 2017
Merged

MAGECLOUD-1193: Enable strict errors #71

merged 3 commits into from
Oct 31, 2017

Conversation

shiftedreality
Copy link
Member

@shiftedreality shiftedreality commented Oct 31, 2017

https://magento2.atlassian.net/browse/MAGECLOUD-1193

Description

This PR add strict errors handling to avoid dummy errors.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • Pull request was approved by architect
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@shiftedreality shiftedreality requested a review from kandy October 31, 2017 10:36
* @return bool
* @throws \Exception
*/
public function handler($errorNo, $errorStr, $errorFile, $errorLine)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this method should be named as "handle"

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed


$msg = isset($this->errorPhrases[$errorNo]) ? $this->errorPhrases[$errorNo] : "Unknown error ({$errorNo})";
$msg .= ": {$errorStr} in {$errorFile} on line {$errorLine}";
throw new \Exception($msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why notice or strict should throw an exception?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fail fast principle

@shiftedreality shiftedreality added Progress: accept PR/issue status and removed Progress: review PR/Issue status labels Oct 31, 2017
@shiftedreality
Copy link
Member Author

QA check is not needed for this PR.

@shiftedreality shiftedreality merged commit 979a25c into magento:develop Oct 31, 2017
@shiftedreality shiftedreality deleted the MAGECLOUD-1193 branch October 31, 2017 15:15
@YPyltiai YPyltiai added the Release: 2002.0.5 ECE-Tools Release label May 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: accept PR/issue status Release: 2002.0.5 ECE-Tools Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants