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

4.x - Add setLogErrorRenderer() to ErrorHandler #2801

Merged
merged 8 commits into from Aug 18, 2019

Conversation

RyanNerd
Copy link
Contributor

@RyanNerd RyanNerd commented Aug 13, 2019

Please review. If the logic looks good I'll add unit tests for this feature.

Closes #2797

Slim/Handlers/ErrorHandler.php Outdated Show resolved Hide resolved
@l0gicgate l0gicgate added this to the 4.2.0 milestone Aug 13, 2019
@adriansuter
Copy link
Contributor

@adriansuter adriansuter commented Aug 14, 2019

The logic looks good to me. We should add unit tests.

@l0gicgate
Copy link
Member

@l0gicgate l0gicgate commented Aug 14, 2019

@adriansuter definitely. @RyanNerd do you need help with writing the tests?

@coveralls
Copy link

@coveralls coveralls commented Aug 14, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 10b11bd on RyanNerd:setLogErrorRenderer into 335b5fd on slimphp:4.x.

@RyanNerd
Copy link
Contributor Author

@RyanNerd RyanNerd commented Aug 17, 2019

@l0gicgate I need to find some time to write the unit tests. Things got busy for me. I'll see if I can get something out there in the next 7 days.

@l0gicgate
Copy link
Member

@l0gicgate l0gicgate commented Aug 18, 2019

@RyanNerd okay no problem. I might just get antsy and write the tests for you if I have 5 mins 😂

@l0gicgate l0gicgate changed the title Add setLogErrorRenderer() to ErrorHandler 4.x - Add setLogErrorRenderer() to ErrorHandler Aug 18, 2019
@l0gicgate l0gicgate merged commit 8ab3432 into slimphp:4.x Aug 18, 2019
2 checks passed
@l0gicgate l0gicgate mentioned this pull request Aug 20, 2019
@RyanNerd RyanNerd deleted the setLogErrorRenderer branch Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants