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 custom alias/name for exception class #153

Merged
merged 2 commits into from
Apr 9, 2020
Merged

Conversation

mvorisek
Copy link
Member

@mvorisek mvorisek commented Apr 9, 2020

Plus use "Critical Error" for non-atk exceptions to be consistent with them.

Before:
image

After:
image

@mvorisek mvorisek requested review from abbadon1334 and romaninsh and removed request for abbadon1334 April 9, 2020 13:11
@romaninsh romaninsh removed their request for review April 9, 2020 13:12
@romaninsh
Copy link
Member

Added by @abbadon1334 , I don't think it's documented though. Not sure what was the intention.

@mvorisek
Copy link
Member Author

mvorisek commented Apr 9, 2020

@romaninsh Having custom name in opposite to the real class name is very confusing. I left there custom_exception_title even it seems to be not used acrosss in atk projects.

@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

Merging #153 into develop will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #153      +/-   ##
=============================================
- Coverage      97.09%   97.06%   -0.04%     
+ Complexity       460      456       -4     
=============================================
  Files             24       24              
  Lines           1067     1055      -12     
=============================================
- Hits            1036     1024      -12     
  Misses            31       31              
Impacted Files Coverage Δ Complexity Δ
src/Exception.php 100.00% <ø> (ø) 19.00 <0.00> (-1.00)
src/ExceptionRenderer/Console.php 100.00% <100.00%> (ø) 24.00 <0.00> (ø)
src/ExceptionRenderer/HTML.php 100.00% <100.00%> (ø) 26.00 <0.00> (ø)
src/ExceptionRenderer/HTMLText.php 100.00% <100.00%> (ø) 24.00 <0.00> (ø)
src/ExceptionRenderer/JSON.php 100.00% <100.00%> (ø) 22.00 <0.00> (ø)
src/ExceptionRenderer/RendererAbstract.php 100.00% <100.00%> (ø) 32.00 <0.00> (-3.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8863a4a...e05265a. Read the comment docs.

Copy link
Collaborator

@abbadon1334 abbadon1334 left a comment

Choose a reason for hiding this comment

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

Agree with @mvorisek, i don't remember who requested it, but was discussed the ability to set a custom name for Exceptions.

For me there is no problem to remove it.

@mvorisek mvorisek merged commit 20d7471 into develop Apr 9, 2020
@mvorisek mvorisek deleted the improve_exception branch April 9, 2020 14:44
@romaninsh
Copy link
Member

romaninsh commented Apr 9, 2020

@mvorisek i don't agree or disagree - just wanted to give a chance to @abbadon1334 and hear his reasoning ;)

Well done on this PR!

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

3 participants