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

[#60][#62] Unchecked Exception in Parser #72

Merged
merged 11 commits into from
Apr 30, 2021
Merged

[#60][#62] Unchecked Exception in Parser #72

merged 11 commits into from
Apr 30, 2021

Conversation

dpeger
Copy link
Contributor

@dpeger dpeger commented Apr 23, 2021

Update maven plugin and contact information to match master pom.xml
Caught unchecked `NumberFormatException` during float parsing and converted into checked `ParseException`.

Fixes #60
Modified test to use Junit4's `Parameterized` runner and added an emoji test
Used `StandardCharsets.UTF_8` to create `String` in `JSONParserByteArray` to avoid dependency on system's default charset
@UrielCh
Copy link
Contributor

UrielCh commented Apr 24, 2021

Looks good, except that you include PR74, this PR in not published in any V2.4 version yet.

@dpeger
Copy link
Contributor Author

dpeger commented Apr 25, 2021

Do you want me to backout the fix for #73 from this PR and create a dedicated PR for this for v2.3 branch? Or do you simply want to wait until PR #74 was released for v2.4?

@UrielCh
Copy link
Contributor

UrielCh commented Apr 25, 2021

PR74 should not be included in branch 2.3; it could cause breathing changes for current users; the parser is now based on OS environment default encoding.

Maybe I'm wrong.

Furthermore, what is your point of view about issue 36? As you may have noticed, I'm not using java anymore...

@dpeger
Copy link
Contributor Author

dpeger commented Apr 26, 2021

I reverted the fix for #73 as this might introduce a breaking change.

@dpeger dpeger requested a review from UrielCh April 27, 2021 10:41
@dpeger
Copy link
Contributor Author

dpeger commented Apr 28, 2021

@UrielCh would you mind to have a second look and merge this PR? I was hoping to see the fixed version in the next spring-boot update...

@UrielCh UrielCh merged commit 7304d1e into netplex:v2.3 Apr 30, 2021
@dpeger dpeger deleted the fixes/v2.3/parse-numberformatexception branch April 30, 2021 07: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