[
https://issues.apache.org/jira/browse/HADOOP-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12634326#action_12634326
]
Chris Douglas commented on HADOOP-4226:
---------------------------------------
bq. I might make one more little change to the code to make it a little
cleaner, but otherwise I think the patch is ready. I urge commiters to look at
it and let me know what they think. I'd hate my time to be wasted, so please
comment.
If you're going to change the code again, a review of the final version is the
best use of everyone's time. Also: it's only been two days. This is a heavily
used piece of code with a lot of corner cases, so this refactoring is risky,
low-priority, and non-trivial to review. Please try to be patient.
bq. Tests: since this is not new code, I don't feel obligated to provide more
tests. There already exist tests for TextInputFormat that are already checking
this code; these tests pass.
TestTextInputFormat is testing LineRecordReader.LineReader, not
o.a.h.util.LineReader, to which this patch applies.
* Per the [guidelines|http://wiki.apache.org/hadoop/HowToContribute], don't use
tabs
* Why track str.getLength() with a local var?
* This shouldn't append stray \r characters to the end of the string. This
should treat \r, \n, and \r\n as delimiters, as in the original. If there isn't
a unit test validating this, writing it would be helpful.
* This appears to regard \r as a valid character (newLineLength is reset for
each char read, and only relevant when followed by \n), but the original does
not.
> LineReader::readLine cleanup
> ----------------------------
>
> Key: HADOOP-4226
> URL: https://issues.apache.org/jira/browse/HADOOP-4226
> Project: Hadoop Core
> Issue Type: Improvement
> Components: mapred
> Affects Versions: 0.19.0
> Reporter: Yuri Pradkin
> Assignee: Yuri Pradkin
> Priority: Minor
> Attachments: HADOOP-4226.patch, HADOOP-4226.patch
>
>
> I've been looking at HADOOP-4010 and realized that readLine is pretty
> convoluted. I changed the implementation which made it hopefully a little
> easier to read/validate/understand.
> I've had some problems testing it locally, so I'll submit it for Hudson to
> test.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
