Skip to content

Support sub sequences in CharSequenceReader#91

Merged
garydgregory merged 6 commits into
apache:masterfrom
robtimus:CharSequenceReaderSubSequence
Apr 28, 2020
Merged

Support sub sequences in CharSequenceReader#91
garydgregory merged 6 commits into
apache:masterfrom
robtimus:CharSequenceReaderSubSequence

Conversation

@robtimus

@robtimus robtimus commented Aug 10, 2019

Copy link
Copy Markdown
Contributor

Depends on PR 90.

@garydgregory

garydgregory commented Aug 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Thank you for the PR.

Some requests:

The Travis build fails with Checkstyle errors, may you fix these?

Create a Jira ticket and link it to this PR.

Update changes.xml with an entry for this PR.

Do the same for your other PR if it represents something different as opposed to an evolution of this PR the user does not need to know about.

Thank you!
Gary

@robtimus

robtimus commented Aug 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

See https://issues.apache.org/jira/browse/IO-619. I've added you as developer just like the other issues. I've also fixed a small bug in changes.xml where you referenced the wrong ticket number (2x 617 instead of 617 and 618).

The other PR is a performance improvement that probably does not warrant its own release notes.

@coveralls

coveralls commented Aug 10, 2019 •
edited
Loading

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.04%) to 89.716% when pulling 176c17e on robtimus:CharSequenceReaderSubSequence into 101f3c2 on apache:master.

garydgregory
garydgregory requested changes Aug 10, 2019
View reviewed changes
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java
@garydgregory garydgregory mentioned this pull request Aug 10, 2019
Merged
@garydgregory

garydgregory commented Nov 5, 2019

Copy link
Copy Markdown
Member

Please rebase on master and omit changes.xml, it makes it easier to merge when dealing with lots of PRs. I should not have asked you to edit changes.xml in the first place! ;-)

@robtimus

robtimus commented Nov 5, 2019

Copy link
Copy Markdown
Contributor Author

Rebase complete, including revert of changes to changes.xml. That does mean that the incorrect 617 (should be 618) on line 131 is back.

@robtimus robtimus requested a review from garydgregory April 13, 2020 12:32
garydgregory
garydgregory reviewed Apr 26, 2020
View reviewed changes
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
@robtimus robtimus requested a review from garydgregory April 26, 2020 19:28
@garydgregory

garydgregory commented Apr 26, 2020

Copy link
Copy Markdown
Member

Hi @robtimus The PR is messed up now, it looks like you did a merge instead of a rebase. Please fix.

@robtimus

robtimus commented Apr 27, 2020

Copy link
Copy Markdown
Contributor Author

Hi @robtimus The PR is messed up now, it looks like you did a merge instead of a rebase. Please fix.

Done. I ended up doing a cherry-pick from master instead, but the results are the same.

garydgregory
garydgregory requested changes Apr 27, 2020
View reviewed changes

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robtimus TY for your patience...

Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
Comment thread src/main/java/org/apache/commons/io/input/CharSequenceReader.java Outdated
@robtimus robtimus requested a review from garydgregory April 28, 2020 18:54
@garydgregory garydgregory merged commit 39e3ea7 into apache:master Apr 28, 2020
asfgit pushed a commit that referenced this pull request Apr 28, 2020
@garydgregory
Support sub sequences in CharSequenceReader #91.
b0bf3d9
brettlounsbury pushed a commit to brettlounsbury/commons-io that referenced this pull request May 28, 2020
@robtimus
Support sub sequences in CharSequenceReader (apache#91)
80c26e8 
* Added support for sub sequences in CharSequenceReader

* Added missing Javadoc. Added entry to changes.xml.

* Added missing @SInCE tags.

* [IO-619] Fixed issues reported in merge request apache#91

* [IO-619] Use assertThrows instead of try-fail-catch

* [IO-619] Fixed issues reported in merge request apache#91
brettlounsbury pushed a commit to brettlounsbury/commons-io that referenced this pull request May 28, 2020
@garydgregory
Support sub sequences in CharSequenceReader apache#91.
38abd1d
@robtimus robtimus deleted the CharSequenceReaderSubSequence branch July 8, 2020 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@garydgregory garydgregory Awaiting requested review from garydgregory

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robtimus @garydgregory @coveralls