corrected UncompressInputStream#718
Merged
josemduarte merged 8 commits intobiojava:masterfrom Jan 30, 2018
Merged
Conversation
Fixed bugs where final bytes of file were not decoded
in available() and the EOF condition for mainloop.
Added some comments
Indented some unindented lines
Added method uncompress(InputStream,OutputStream)
and called it from main(String[])
and from uncompress(String, FileOutputStream)
Rewrote skip method
Amended logging code in uncompress(String, FileOutputStream)
tweaked skip()
Contributor
|
Thanks! travis is complaining about a compilation error. Any chance that something was missed in the commits? |
Contributor
Author
|
I may have used some Java 1.8 feature. What level of Java is appropriate? Is there a way for me to see Travis's output? (I looked, but only found "travis-settings.xml")
Fred Hansen
On Tuesday, January 9, 2018, 2:27:49 PM EST, Jose Manuel Duarte <notifications@github.com> wrote:
Thanks! travis is complaining about a compilation error. Any chance that something was missed in the commits?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Contributor
|
This is the error: In the master branch we are in java 8 (for upcoming release 5.0.0), so there shouldn't be an issue there. |
Typo in skip() - fix compile error
Contributor
Author
|
Sigh. Tried to do a change via shortcut and deleted a parenthesis. I've revised the code but not yet done a new pull. I am going to to test the revised method.
On Tuesday, January 9, 2018, 2:46:39 PM EST, Jose Manuel Duarte <notifications@github.com> wrote:
This is the error:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.5.1:compile (default-compile) on project biojava-core: Compilation failure
[ERROR] /home/travis/build/biojava/biojava/biojava-core/src/main/java/org/biojava/nbio/core/util/UncompressInputStream.java:[384,61] ')' expected
In the master branch we are in java 8 (for upcoming release 5.0.0), so there shouldn't be an issue there.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
josemduarte
requested changes
Jan 9, 2018
Contributor
josemduarte
left a comment
josemduarte
left a comment
There was a problem hiding this comment.
Changes look good, thanks a lot. My only request would be to add a unit test for this under biojava-core/src/test/java. The test should also cover the issue that is solved here.
Contributor
Author
|
I should learn JUnit and writing this test would be a great opportunity. But I find myself in the position of having been beguiled by too many opportunities. And now I find myself in a time crunch. Sigh. I regret that I cannot build a test at this time. (I might have been ale to revise one, but mine would be the first.
On Tuesday, January 9, 2018, 3:29:39 PM EST, Jose Manuel Duarte <notifications@github.com> wrote: @josemduarte requested changes on this pull request.
Changes look good, thanks a lot. My only request would be to add a unit test for this under biojava-core/src/test/java. The test should also cover the issue that is solved here.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Contributor
|
I've just committed a test to a branch and have pull requested it to your branch. If you merge my pull request zweibieren#1 then my commit will be added to this PR. The test I wrote actually does not pass, the uncompressed stream has an extra new line character at the end. Could you review the test and check what's going on? Thanks! |
Contributor
Author
|
fixed a test and added another |
Contributor
|
Did you want to create a new pull request for this? or was it just closed by mistake? This one is ready to be merges as it is. |
Contributor
Author
|
I'm new to pull requests and probably did it wrong. I'm happy with it all. You can merge or tell me what to do.
Fred Hansen
From: Jose Manuel Duarte <notifications@github.com>
To: biojava/biojava <biojava@noreply.github.com>
Cc: Fred Hansen <zweibieren@yahoo.com>; State change <state_change@noreply.github.com>
Sent: Monday, January 29, 2018 12:36 PM
Subject: Re: [biojava/biojava] corrected UncompressInputStream (#718)
Did you want to create a new pull request for this? or was it just closed by mistake?This one is ready to be merges as it is.—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Contributor
|
No problem. I'll merge this one then. The convention is to close a PR when it's merged or otherwise if there's no consensus that it should be merged. |
josemduarte
approved these changes
Jan 30, 2018
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The original UncompressInputStream could omit the final byes from the original. This patch corrects that. It also adds a new method for uncompressing from one stream to another; two existing methods have been revised to use this new method. Indentation was improved and a few comments added.