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

Revert recent obj long get long changes #869

Merged
merged 3 commits into from Feb 28, 2024

Conversation

stleary
Copy link
Owner

@stleary stleary commented Feb 25, 2024

See #868.
Reverts recent code changes that break existing applications.
This PR will be left up for a few days for comments. If there are no objections or concerns it will included in a new release soon.

For more information, please see XMLTest.clarifyCurrentBehavior() and JSONObjectTest.clarifyCurrentBehavior().

@stleary
Copy link
Owner Author

stleary commented Feb 28, 2024

What problem does this code solve?
Reverts some recently released changes that do not work as intended. Changes to improve optLong vs getLong are needed, but must not change existing functionality without good reason. In this case, strings consisting of some number of leading zeros followed by any digits should not be interpreted as numbers, as that goes against the JSON spec.

Does the code still compile with Java6?
Yes

Risks
Moderate. The faulty code is already released, and may potentially already be in use. On the other hand, the last release was relatively recent, and the misbehaving code is a bit of an edge case.

Changes to the API?
getLong(), optLong(), and stringToNumber() functionality is reverted.

Will this require a new release?
Yes. However, there are several workflow issues that should be addressed before the release:

  • Java 11 builds tend to fail due to parallel execution. It should be executed separately. Actually, all of the executions should be separate.
  • The deployment workflow is redundant at the moment, since the actual deployment is still a manual process. The workflow should be silenced until the transition from manual process has been completed.

Should the documentation be updated?
No

Does it break the unit tests?
No. Some recently added unit tests are removed.

Was any code refactored in this commit?
No

Review status
APPROVED - by myself

In this case, the review window has already completed.

A release will be performed as soon as the workflows are updated.

@stleary stleary merged commit 3f97826 into master Feb 28, 2024
12 checks passed
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

1 participant