Skip to content
This repository was archived by the owner on Sep 8, 2021. It is now read-only.

Revert escription of a exception in JaudiotaggerParser #1626

Merged
fxthomas merged 1 commit into
masterfrom
revert-exception
Apr 19, 2020
Merged

Revert escription of a exception in JaudiotaggerParser #1626
fxthomas merged 1 commit into
masterfrom
revert-exception

Conversation

@tesshucom
Copy link
Copy Markdown
Contributor

@tesshucom tesshucom commented Apr 17, 2020

This is a revert for the #1625.
Due to lack of verification, we will also revert exception descriptions other than those reported.

It is a revert (patch) to instant recover the deterioration that cannot be untraceable by the user.
Another new exception description, logging and test addition (refactoring) should be done in another PR.

Note : CHANGELOG.md

Exception handling of JaudiotaggerParser will be reverted to v10.5.0
base.

related c6eed14, a13552b
@tesshucom
Copy link
Copy Markdown
Contributor Author

tesshucom commented Apr 17, 2020

We have created a minimal patch based on legacy so that people in need can apply it.
Please add as necessary, create a more appropriate commit and merge.
There is no problem if you leave it as it is.

try{
}catch(~Exception e) {  <=Add!
}catch(~Exception e) {  <=Add!
}catch(~Exception e) {  <=Add!
}catch(~Tthrowable e) {
}

For Java, If want to separate logs, just add them in exeption level order.

I'm having deja vu.
(In another commit, I feel like I had a similar story with randomnicode and fxthomas...)


  • The reason why the exception design of Jaudiotagger is high abstract is that it is a specification that reflects the product characteristics.
  • It's not laziness that JaudiotaggerParser has a huge try block, it should be (Logging aggregation).

@fxthomas fxthomas added in: UNKNOWN-backend The problem location has not been identified yet. type: regression If something that was operating normally goes deterioration, it will be accumulated as a incident. labels Apr 17, 2020
Copy link
Copy Markdown
Contributor

@fxthomas fxthomas left a comment

Choose a reason for hiding this comment

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

Looks good to me, I think a 10.6.1 will be needed. Will try to create one this week-end.

We will need to add tests cases also, but this can be done later. I will keep the issue open so that we don't forget it. Let me know if you have time to create a PR for them.

@tesshucom
Copy link
Copy Markdown
Contributor Author

I think this way.

  • If it's a regressive repair, having a test is not ''urgent''. Because it have a very long track record already. Of course, there should be tests. But it doesn't exist in legacy, and it's quite difficult to achieve. If realized, it will overlap with the work of the Jaudiotagger project.
  • After applying this PR (that is, after reverting to the traditional implementation), any further exception design change requires valid explanation and testing. Because a defect track record has already been created.

So, I think it's okay if you include this PR simply when you release 10.6.1.
After that, you need to be alert when a similar PR comes.

@fxthomas fxthomas mentioned this pull request Apr 18, 2020
9 tasks
@fxthomas fxthomas added this to the 10.6.1 milestone Apr 18, 2020
@fxthomas fxthomas merged commit 942542a into master Apr 19, 2020
@fxthomas fxthomas deleted the revert-exception branch April 19, 2020 10:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

in: UNKNOWN-backend The problem location has not been identified yet. type: regression If something that was operating normally goes deterioration, it will be accumulated as a incident.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

java.lang.UnsupportedOperationException: Not implemented for this format

2 participants