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

Throw ResourceNotFoundException outside of re-defined error-handler #273

Conversation

dmitry-varennikov-eventbase
Copy link
Contributor

Throwing exception outside of re-defined error-handler allows us to catch it in our app stack

@Maks3w
Copy link
Contributor

Maks3w commented May 25, 2016

Please avoid all those formatting changes, are noisy,

Provide a unit test.

});
$response = file_get_contents($uri);
restore_error_handler();

if ($error) {
list (, $errstr, $errfile, $errline) = $error;
$message = sprintf('%s on line %s in %s', $errstr, $errline,
Copy link
Contributor

Choose a reason for hiding this comment

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

Almost everything in the proposed message template is already present in the stacktrace. $errstr is the only value you need here.

@dmitry-varennikov-eventbase
Copy link
Contributor Author

Unit test JsonSchema\Tests\Uri\Retrievers\FileGetContentsTest::testFetchMissingFile

@bighappyface
Copy link
Collaborator

@Maks3w
Copy link
Contributor

Maks3w commented May 26, 2016

This is not about visualization. It's about to keep a clean and organized history.

If the CS changes was done in a different PR you didn't have to revert.

@bighappyface
Copy link
Collaborator

@Maks3w I'm starting to question the value of considering your input when you basically have nothing good or nice to say about anything and have contributed little to this project but hyper-criticism.

How about you https://m.spreadshirt.com/shut-the-fuck-up-and-write-some-code-t-shirts-A103693056

@jojo1981
Copy link

@Maks3w What about the "The Boy Scout Rule"? Why you're complaining about this nice PR?

@dmitry-varennikov-eventbase
Copy link
Contributor Author

Does this PR require any more code?
Is this PR could be merged any time soon?

@bighappyface
Copy link
Collaborator

+1

@bighappyface
Copy link
Collaborator

Once this is merged does anyone see any issue with bumping the package to 2.0.5?

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.

None yet

4 participants