Skip to content

Conversation

@jglick
Copy link
Member

@jglick jglick commented Feb 13, 2018

Integrates #60 & #61. Please leave line comments in one of the parents until you are referring specifically to something about the integration.

@reviewbybees

Copy link
Member

@svanoort svanoort left a comment

Choose a reason for hiding this comment

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

Blocking until can do a deep review/test.

@oleg-nenashev
Copy link
Member

@jglick I have approved #61. #60 is WiP if I understand your comment correctly. I'd guess no need to review this PR now.

@oleg-nenashev oleg-nenashev removed their request for review April 2, 2018 13:37
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Requesting changes to reflect the current status of #60

@jglick
Copy link
Member Author

jglick commented Jun 9, 2018

Please do not perform line-by-line reviews in this PR. Rather, review component PRs.

@jglick jglick requested a review from svanoort July 9, 2018 20:25
try (FileChannel ch = FileChannel.open(Paths.get(logFile.getRemote()), StandardOpenOption.READ)) {
InputStream locallyEncodedStream = Channels.newInputStream(ch.position(lastLocation));
InputStream utf8EncodedStream = cs == null ? locallyEncodedStream : new ReaderInputStream(new InputStreamReader(locallyEncodedStream, cs), StandardCharsets.UTF_8);
CountingInputStream cis = new CountingInputStream(utf8EncodedStream);
Copy link
Member Author

Choose a reason for hiding this comment

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

This probably does the same miscount when transcoding as #61, to be confirmed.

@jglick jglick requested a review from dwnusbaum August 6, 2018 20:37
@jglick
Copy link
Member Author

jglick commented Aug 7, 2018

Closing as this is now identical to #60.

@jglick jglick closed this Aug 7, 2018
@jglick jglick deleted the watch-plus-UTF-8-JENKINS-31096 branch August 7, 2018 15:20
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.

4 participants