Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change LzoTextLine() to LzoTextLine('line)? #817

Open
tanin47 opened this issue Mar 9, 2014 · 3 comments
Open

Change LzoTextLine() to LzoTextLine('line)? #817

tanin47 opened this issue Mar 9, 2014 · 3 comments

Comments

@tanin47
Copy link

tanin47 commented Mar 9, 2014

Here: https://github.com/twitter/scalding/blob/develop/scalding-commons/src/main/scala/com/twitter/scalding/commons/source/LzoTraits.scala#L72

It is because new LzoTextLine() will call TextLine's default constructor, which will return 2 columns: offset and line (http://docs.cascading.org/cascading/2.1/javadoc/cascading/scheme/hadoop/TextLine.html)

Please let me know what you think

@johnynek
Copy link
Collaborator

So, is the code currently broken? It seems like it is from your description. Can we confirm with a unit test, somehow?

Ideally, I'd love a test that is currently red, make PR, then a fix that gets it green. That would be rad.

@tanin47
Copy link
Author

tanin47 commented Mar 10, 2014

From what, I've found, yes.

Let me add a test case that verifies the problem. Thanks for the
suggestion, Oscar!

-Tanin

On Mon, Mar 10, 2014 at 10:48 AM, P. Oscar Boykin
notifications@git.luolix.topwrote:

So, is the code currently broken? It seems like it is from your
description. Can we confirm with a unit test, somehow?

Ideally, I'd love a test that is currently red, make PR, then a fix that
gets it green. That would be rad.

Reply to this email directly or view it on GitHubhttps://github.com//issues/817#issuecomment-37212128
.

@tanin

@tanin47 tanin47 self-assigned this Mar 16, 2014
@jcoveney
Copy link
Contributor

Closed by #921

@tanin47 tanin47 removed their assignment Mar 17, 2015
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

No branches or pull requests

3 participants