Skip to content

GROOVY-8251: Implement withCloseable on AutoCloseable#572

Closed
jwagenleitner wants to merge 4 commits intoapache:masterfrom
jwagenleitner:8251-AutoCloseable
Closed

GROOVY-8251: Implement withCloseable on AutoCloseable#572
jwagenleitner wants to merge 4 commits intoapache:masterfrom
jwagenleitner:8251-AutoCloseable

Conversation

@jwagenleitner
Copy link
Copy Markdown
Contributor

@jwagenleitner jwagenleitner commented Jul 9, 2017

NioGroovyMethods.withAutoCloseable was added for 2.5.0 as part of GROOVY-7572. It appears it was originally placed there because at the time Java 6 was still used, but now 2.5.0 is 7+.

First commit 9122001 proposes moving the method (as-is) into the core IOGroovyMethods class. This is where the withCloseable(java.io.Closeable...) method is located. This change also moves the tests for both methods from NIO to core. Since 2.5.0 has not been officially released there may not be a need to deprecate or worry about binary compatibility for the withAutoCloseable method?

Second commit 261bc17 proposes to rename the new but unreleased method from withAutoCloseable to withCloseable to be consistent with the existing method on java.io.Closeable. See discussion on GROOVY-8251.

Relocate withAutoCloseable from NIO subproject to core. Since
AutoCloseable is not strictly an NIO related class and release
2.5.0 will target Java 7, the method should be available as part
of core.

Relocated withCloseable tests to core since that method has
already been moved to core and is deprecated in the NIO module.
AutoCloseables and Closeables should both be handled consistently.
Naming the method withCloseable is general enough to handle both
cases without requiring distinct naming.
@jwagenleitner
Copy link
Copy Markdown
Contributor Author

Updated PR to also include GROOVY-8259 adding suppressed exceptions.

@PascalSchumacher
Copy link
Copy Markdown
Contributor

It appears it was originally placed there because at the time Java 6 was still used, but now 2.5.0 is 7+.

That was the reason (I think I merged the pull request.).

Since 2.5.0 has not been officially released there may not be a need to deprecate or worry about binary compatibility for the withAutoCloseable method?

+1 no need to keep binary compatibility to a beta release.

@paulk-asert
Copy link
Copy Markdown
Contributor

+1 no need to keep binary compatibility with beta-1

@asfgit asfgit closed this in 9d8e433 Jul 18, 2017
@jwagenleitner jwagenleitner deleted the 8251-AutoCloseable branch July 18, 2017 01:49
asfgit pushed a commit that referenced this pull request Jul 18, 2017
Relocate withAutoCloseable from NIO subproject to core. Since
AutoCloseable is not strictly an NIO related class and release
2.5.0 will target Java 7, the method should be available as part
of core.

Relocated withCloseable tests to core since that method has
already been moved to core and is deprecated in the NIO module.
asfgit pushed a commit that referenced this pull request Jul 18, 2017
Relocate withAutoCloseable from NIO subproject to core. Since
AutoCloseable is not strictly an NIO related class and release
2.5.0 will target Java 7, the method should be available as part
of core.

Relocated withCloseable tests to core since that method has
already been moved to core and is deprecated in the NIO module.
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.

3 participants