Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
garydgregory commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4445637724 Hello @mehmet-karaman Thank you for your patience. This is on my list. I am evaluating other PRs and issues. TY! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4445183828 Hi @garydgregory , Just wanted to ping again :). Is there any chance to bring this PR on its way? Best regards Mehmet -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
garydgregory commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4250683682 @uvoigt It's not forgotten. There are other tasks that are higher priority than new features 😉 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4250627680 Hi @garydgregory, I hope you’re doing well. Just a quick friendly reminder before this gets out of scope :) Thanks in advance! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
uvoigt commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4178886047 > Hello @mehmet-karaman > > Please be patient. This is not the only issue or Commons component that needs attention 😉 Happy Easter! Hope you're enjoying some free time! Thank you for your work on these kinds of software projects! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
garydgregory commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4175305278 Hello @mehmet-karaman Please be patient. This is not the only issue or Commons component that needs attention 😉 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4175130209 Hi @garydgregory , do you need anything for this PR, or is something still missing? Best regards Mehmet -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
uvoigt commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4155882918 Fixed a problem with the builder construction. I was not that familiar with `AbstractBuilder`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4154960843 From my perspective, this looks good. All workarounds and edge cases seem to be covered and the implementation as well as the Javadocs are clear and understandable. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
uvoigt commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008699730
##
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java:
##
@@ -929,7 +1134,7 @@ public void finish() throws IOException {
}
if (entry != null) {
-throw new ArchiveException("This archive contains unclosed
entries.");
+closeArchiveEntry();
Review Comment:
It is tested in [testAutoCompressZstd(ZipMethod
zipMethod)](https://github.com/apache/commons-compress/pull/765/changes#diff-540c440c3632bdafdb31449fccac357441aae5f8fd8689e8a17922a4e6caf7bdR43)
for `auto-compress` and in
[testAutoCompressFalsePreservesLegacyBehavior()](https://github.com/apache/commons-compress/pull/765/changes#diff-540c440c3632bdafdb31449fccac357441aae5f8fd8689e8a17922a4e6caf7bd)
for user specified compressing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008656359
##
src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java:
##
@@ -61,10 +61,15 @@ private void doCallSequence(final String archiveType)
throws Exception {
aos2.putArchiveEntry(aos2.createArchiveEntry(dummy, "dummy"));
aos2.write(dummy);
-// TODO check if second putArchiveEntry() can follow without closeAE?
Review Comment:
This one can also be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008654655
##
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java:
##
@@ -929,7 +1134,7 @@ public void finish() throws IOException {
}
if (entry != null) {
-throw new ArchiveException("This archive contains unclosed
entries.");
+closeArchiveEntry();
Review Comment:
We can close this.. There was a todo in the ArchiveOutputStreamTest which is
resolved by this change, so it was somehow planned to do..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008536629
##
src/test/java/org/apache/commons/compress/archivers/ArchiveOutputStreamTest.java:
##
@@ -61,10 +61,15 @@ private void doCallSequence(final String archiveType)
throws Exception {
aos2.putArchiveEntry(aos2.createArchiveEntry(dummy, "dummy"));
aos2.write(dummy);
-// TODO check if second putArchiveEntry() can follow without closeAE?
Review Comment:
Ah.. sorry didn't saw that there is a todo.. Might be also a good time to
fix this..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008423250
##
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java:
##
@@ -929,7 +1134,7 @@ public void finish() throws IOException {
}
if (entry != null) {
-throw new ArchiveException("This archive contains unclosed
entries.");
+closeArchiveEntry();
Review Comment:
To keep the API compatible with older clients, wouldn’t it be better to
execute this only when the isAutoCompressed flag is set and otherwise still
throw the exception?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008423250
##
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java:
##
@@ -929,7 +1134,7 @@ public void finish() throws IOException {
}
if (entry != null) {
-throw new ArchiveException("This archive contains unclosed
entries.");
+closeArchiveEntry();
Review Comment:
To keep the API compliant for older clients, wouldn't it be better to do
this only if isAutoCompressed flag is set?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on code in PR #765:
URL: https://github.com/apache/commons-compress/pull/765#discussion_r3008423250
##
src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveOutputStream.java:
##
@@ -929,7 +1134,7 @@ public void finish() throws IOException {
}
if (entry != null) {
-throw new ArchiveException("This archive contains unclosed
entries.");
+closeArchiveEntry();
Review Comment:
Wouldn't it be better to do this only if isAutoCompressed flag is set?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [COMPRESS-722] Introduce handling of entries compressed by a specific method [commons-compress]
mehmet-karaman commented on PR #765: URL: https://github.com/apache/commons-compress/pull/765#issuecomment-4153015587 @garydgregory is it possible to add copilot to the reviewers list to have a raw check? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
