Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

JSONTokener implemented java.io.Closeable #733

Merged
merged 2 commits into from
Mar 12, 2023

Conversation

haribabu-dev
Copy link
Contributor

No description provided.

@stleary
Copy link
Owner

stleary commented Mar 4, 2023

@haribabu-dev Why should JSONTokener implement Closeable? It is not a closeable resource. Can you just add the close() method without the override or interface?

@haribabu-dev
Copy link
Contributor Author

Ok @stleary , I will do that.

@stleary
Copy link
Owner

stleary commented Mar 9, 2023

What problem does this code solve?
Users may want to close the reader associated with a JSONTokener

Risks
Low

Changes to the API?
A new public API function was added, with unit test

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No

Was any code refactored in this commit?
No

Review status
APPROVED

Starting 3-day comment window

@stleary stleary merged commit fe22b24 into stleary:master Mar 12, 2023
@jtotht
Copy link

jtotht commented Apr 11, 2023

@haribabu-dev Why should JSONTokener implement Closeable? It is not a closeable resource. Can you just add the close() method without the override or interface?

Because it (potentially) uses closable resources. See #718, which would have been closed by the original version of this PR, but not the one that got merged.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants