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

Remove redundant error handler in generate #689

Open
wants to merge 1 commit into
base: 2.15.x
Choose a base branch
from

Conversation

michalbundyra
Copy link
Contributor

@michalbundyra michalbundyra commented Apr 19, 2021

Relates to webimpress/safe-writer#49

The problem is with testing in ocramius/proxy-manager, not in safe-writer itself.

Update: still it can be improved in safe-writer to set custom error-handler instead of suppressing warnings with @.

Relates to webimpress/safe-writer#49

The problem is with testing in ocramius/proxy-manager, not in safe-writer
itself.
@Ocramius
Copy link
Owner

Aha, I guess the @ part really was the catch 👍

Still, won't that trip tools like sentry, which declare custom error handlers to catch these cases? 🤔

@@ -44,16 +37,12 @@ public function generate(ClassGenerator $classGenerator): string
$className = $classGenerator->getNamespaceName() . '\\' . $classGenerator->getName();
$fileName = $this->fileLocator->getProxyFileName($className);

set_error_handler($this->emptyErrorHandler);
Copy link
Owner

Choose a reason for hiding this comment

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

Discussed in chat: change is valid, I just want to merge the removal of the error handlers separately from the test adjustment first, but overall 👍

@Ocramius Ocramius self-assigned this Apr 19, 2021
@Ocramius Ocramius added this to the 2.12.0 milestone Apr 19, 2021
@Ocramius Ocramius modified the milestones: 2.12.0, 2.13.0 May 25, 2021
@Ocramius Ocramius modified the milestones: 2.13.0, 2.14.0 Jun 9, 2021
@Ocramius Ocramius changed the base branch from 2.12.x to 2.14.x July 23, 2021 18:53
@Ocramius Ocramius self-requested a review February 22, 2022 15:18
@Ocramius Ocramius modified the milestones: 2.14.0, 2.15.0 Feb 28, 2022
@Ocramius Ocramius changed the base branch from 2.14.x to 2.15.x February 28, 2022 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants