jodatime re-sending to reflect latest changes. #136
Conversation
date classes to support joda time and other date conversions.
lazy val for bijectionJodaTime
| package com.twitter.bijection | ||
|
|
||
| import java.util._ | ||
| import com.github.nscala_time.time._ |
There was a problem hiding this comment.
you are still importing joda in the bijection package in bijection-core.
Does that not show up in your view of this?
There was a problem hiding this comment.
you are correct. I removed both classes from the core package.
| override def invert(s: String) = attempt(s)(new DateTime(_)) | ||
| } | ||
|
|
||
| implicit val joda2Long: Injection[DateTime, Long] = |
There was a problem hiding this comment.
isn't this a bjiection? getMillis never throws, and construction from Long never throws, right?
There was a problem hiding this comment.
You are correct the [getMillis] (http://joda-time.sourceforge.net/apidocs/org/joda/time/base/BaseDateTime.html#getMillis() never throws neither does the Long constructor. Moving this to a bijection
…into develop Conflicts: bijection-core/src/main/scala/com/twitter/bijection/DateInjections.scala.orig
|
I am working on the test cases , and not sure what am I doing wrong , package com.twitter.bijection.jodatime
import org.scalacheck.Properties
import org.scalacheck.Gen._
import org.scalacheck.Arbitrary
import org.scalacheck.Prop._
import com.twitter.bijection.{ Bijection,BaseProperties , ImplicitBijection}
import java.util.Date
import org.joda.time.DateTime
import com.twitter.bijection._
object DateBijectionsLaws extends Properties("DateBijections") with BaseProperties {
implicit val Long : Arbitrary [Long] = arbitraryViaFn { (s: DateTime) => (s.getMillis()) }
property("Joda <=> Long") = isBijection[DateTime, Long]
implicit val DateTime : Arbitrary [DateTime] = arbitraryViaFn { (s: Date) => ( new DateTime (s) ) }
property("Date <=> Joda") = isBijection[Date, DateTime]
}I keep getting the error Any idea ? |
|
Yeah, you need to mix in the trait you wrote, otherwise the bijections won't actually be in scope. |
|
@sritchie done. it is merged. |
| @@ -1,15 +1,33 @@ | |||
| # Bijection # | |||
|
|
|||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
these are merge conflict notices.
Take a look at this:
http://genomewiki.ucsc.edu/index.php/Resolving_merge_conflicts_in_Git
|
@johnynek thanks for the feedback ... all done. |
| @@ -6,6 +6,16 @@ the inverse has the same property. | |||
|
|
|||
| See the [current API documentation](http://twitter.github.com/bijection) for more information. | |||
|
|
|||
| <<<<<<< HEAD | |||
There was a problem hiding this comment.
These still need to be removed.
|
So, the main code here is really useful; it looks like we're getting blocked on some really basic issues having to do with git. This issue will be ready to merge when you can click the "Files Changed" link at the top of the issue and see only the changes that you want. If there's extra stuff (like these >>>>>MASTER notes), you need to delete them and re-push to the branch. Once this is all done and the build passes, we'll be happy to take the code. The other option (if this is dragging on too long) is to let me make a separate pull request to show you what it should look like. I'm not sure that this will maximize learning, but I'll leave that up to you. How do you want to move forward? |
|
@sritchie sorry about that , I did a "git status" before pushing and I got nothing. Would you kindly check the latest build. |
| osgiExportAll("com.twitter.bijection.jodatime"), | ||
| libraryDependencies ++= Seq( | ||
| "joda-time" % "joda-time" % "2.2", | ||
| "org.joda" % "joda-convert" % "1.3.1" |
There was a problem hiding this comment.
yes absolutely we do need this dependency, otherwise the build will fail. (http://stackoverflow.com/questions/13856266/class-broken-error-with-joda-time-using-scala)
|
One last request to remove an extra dependency if it's not needed. Then let's merge! |
jodatime re-sending to reflect latest changes.
|
Merged. Thanks for this! |
I closed the last pull request and re-submitted ... please let me know if you see the package structure change.