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

Ianoc/scalding 210 #1116

Merged
merged 24 commits into from
Dec 6, 2014
Merged

Ianoc/scalding 210 #1116

merged 24 commits into from
Dec 6, 2014

Conversation

ianoc
Copy link
Collaborator

@ianoc ianoc commented Dec 5, 2014

Brings @jcoveney and my old stuff up to date

@johnynek
Copy link
Collaborator

johnynek commented Dec 5, 2014

This includes #1059 ?

@@ -348,7 +350,7 @@ object ScaldingBuild extends Build {
lazy val scaldingJson = module("json").settings(
libraryDependencies <++= (scalaVersion) { scalaVersion => Seq(
"org.apache.hadoop" % "hadoop-core" % hadoopVersion % "provided",
"com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.2.3"
"com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.4.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

pull this up to our section on versions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we do that for all versions? there is still a sprinkling of them around. I think most that are common are moved out but we haven't moved once off's before

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we should for all the versions so it easy to see for a reader what things depend on.

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like it, but unless you object a follow up PR? just to get this in and make that one seem clearer

@ianoc
Copy link
Collaborator Author

ianoc commented Dec 5, 2014

Includes Jco's work, but disables the 2.11 repl targets/tests since there are issues there

"org.scalacheck" %% "scalacheck" % "1.11.5" % "test",
"org.scala-tools.testing" %% "specs" % "1.6.9" % "test",
"org.mockito" % "mockito-all" % "1.8.5" % "test"
"org.mockito" % "mockito-all" % "1.8.5" % "test",
Copy link
Collaborator

Choose a reason for hiding this comment

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

didn't even know we were using this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, not sure why/how it got in there really. Just started appearing in all my diffs

@@ -296,6 +296,9 @@ trait TypedPipe[+T] extends Serializable {
def filter(f: T => Boolean): TypedPipe[T] =
flatMap { t => if (f(t)) Iterator(t) else Iterator.empty }

// This is just to appease for comprehension
def withFilter(f: T => Boolean): TypedPipe[T] = filter(f)
Copy link
Contributor

Choose a reason for hiding this comment

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

lets check if typed pipe actually works in a for comprehension

@johnynek
Copy link
Collaborator

johnynek commented Dec 6, 2014

This is big enough in my view. shipit.

We can follow on with the issues Ian added.

Alex if you think it is fine, I'll let you merge.

isnotinvain added a commit that referenced this pull request Dec 6, 2014
Add cross compile support for scala 2.11
@isnotinvain isnotinvain merged commit 7be2776 into develop Dec 6, 2014
@jcoveney
Copy link
Contributor

jcoveney commented Dec 8, 2014

happy tears

@ianoc ianoc deleted the ianoc/scalding_210 branch May 13, 2015 15:32
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.

None yet

4 participants