On Tue, Oct 20, 2009 at 7:01 AM, Derek Chen-Becker <dchenbec...@gmail.com>wrote:
> Here is my argument for breaking it out: > > I think that we're in agreement that the *default* impl for Lift should > eventually be Joda Time, after some period of deprecation on the old stuff. What I checked in allows you to use JodaTime just as easily (well with 2 extra characters in a few method names) as java.util.Date. How is anything more "default" than that? > I also agree with your earlier comment that it should be something that the > developer can choose. I'm sure there will be cases where people want to use > java.util instead of Joda Time because of preference or constraint (say, no > external jars). The problem is that "import Helpers._" is a very common > import, and by having TimeHelpers be part of Helpers it makes it very > difficult to do a clean separation of impls. It's an interesting difference between an OO vs. non-OO. In the implementation I created, there choice of one or the other is made based on singleton methods invoked. This allows mixing both in the same code simply by invoking now or jtNow. > My main example is the time manipulation DSL. We can't mask the implicit > defs that are already part of TimeHelpers, so it's very difficult (maybe > impossible) to define a Joda Time based DSL (which would properly handle > things like daylight savings time, etc). I'm unclear why this is not possible. We can add a DSL for manipulating JodaTime without breaking anything we have. The TimeSpan class simply gets enhanced to deal with additional stuff and maybe uses JodaTime under the covers. > By removing TimeHelpers from Helpers, it means that the user can have > absolute control over which time handling impl they're using without having > to give up all of the nice things in Helpers. They have that now with the implementation I did on your branch. > It would be a breaking change to code, but it would be pretty minor, since > the fix is just to add an import to the code that breaks. Here is my > proposal for how we change things: > > > - *version 1.1-final*: > > > 1. Rename the current TimeHelpers trait to JavaTimeHelpers and add a > new TimeHelpers trait that extends JavaTimeHelpers, providing an alias. > This > means no breaking code in this release. > 2. Create a new JodaTimeHelpers that mirrors the non-deprecated > functionality of JavaTimeHelpers (there were already some deprecated > methods > on TimeHelpers that could go away). > 3. We can also copy back some of the new methods on JodaTimeHelpers > and refactor them for java.util (I'm thinking epoch and some other minor > ones). > 4. Add a JavaTimeFormatting and JodaTimeFormatting trait (currently > TimeRules in my code) that allows for custom formats and format > functions of > dates and times. The reason I want to split out the impls here, just > like > Java/JodaTimeHelpers, is so that Joda can be an optional dependency for > people who can't use it. > > > - *version 1.2-final*: > > > 1. Remove the TimeHelpers trait and remove TimeHelpers from Helpers. At > this point, people would have to explicitly choose JavaTimeHelpers or > JodaTimeHelpers in their import statement to determine which impl they're > using. > > The main thing is that having TimeHelpers tied to Helpers makes it very > difficult to give the user a choice of impls, something that I feel strongly > goes against how Lift does things otherwise. > > Derek > > > On Mon, Oct 19, 2009 at 10:36 PM, David Pollak < > feeder.of.the.be...@gmail.com> wrote: > >> I have a strong preference not to break the TimeHelpers out of Helpers. >> I'm not seeing the problem with additional methods on TimeHelpers that do >> the JodaTime thing. Can you guys help me understand your point of view. >> >> >> On Mon, Oct 19, 2009 at 8:16 PM, Derek Chen-Becker <dchenbec...@gmail.com >> > wrote: >> >>> Oh, that was a sidetrack. I was thinking that it could help enforce the >>> common contract, but return types are different for the same method names so >>> that really wouldn't work. I really just want to try and separate them out, >>> or provide a different "Helpers" that provides JodaTime stuff under the same >>> method names. My goal is really to minimize code changes, so one or two >>> imports instead of a lot of find/replace (as simple as that may be) would be >>> preferable in my book. >>> >>> Derek >>> >>> >>> On Mon, Oct 19, 2009 at 8:53 PM, Naftoli Gugenheim <naftoli...@gmail.com >>> > wrote: >>> >>>> >>>> What would be the purpose of having a common trait though? >>>> >>>> ------------------------------------- >>>> Derek Chen-Becker<dchenbec...@gmail.com> wrote: >>>> >>>> That was pretty much what I was trying to communicate with my last >>>> email, >>>> just not very effectively. >>>> >>>> Derek >>>> >>>> On Mon, Oct 19, 2009 at 7:14 PM, Naftoli Gugenheim < >>>> naftoli...@gmail.com>wrote: >>>> >>>> > >>>> > Since there are anyway minor breaking changes in 1.1, maybe it's worth >>>> it >>>> > to take TimeHelpers out of Helpers. This way, it will simply require >>>> an >>>> > extra import, of either TimeHelpers or JodaHelpers, which can be >>>> chosen by >>>> > the individual developer. >>>> > Whenever someone is ready to migrate, they will be able to do so on a >>>> > file-by-file (or import-scope-by-import-scope) basis. >>>> > Eventually JodaHelpers could be included in Helpers. >>>> > >>>> > >>>> > ------------------------------------- >>>> > Derek Chen-Becker<dchenbec...@gmail.com> wrote: >>>> > >>>> > Along those same lines, maybe there should be a common trait that >>>> doesn't >>>> > define an impl, and then have separate Joda and java.util impl traits >>>> that >>>> > don't mix directly into Helpers. >>>> > >>>> > On Mon, Oct 19, 2009 at 6:57 PM, Derek Chen-Becker < >>>> dchenbec...@gmail.com >>>> > >wrote: >>>> > >>>> > > What I was thinking earlier is that we can simply make a JodaHelpers >>>> > object >>>> > > that mixes in JodaTimeHelpers instead of TimeHelpers. That way, code >>>> > changes >>>> > > to move to Joda Time would mostly just be an import change instead >>>> of >>>> > having >>>> > > to alter every instance of now ⇒ jtNow. >>>> > > >>>> > > Derek >>>> > > >>>> > > >>>> > > On Mon, Oct 19, 2009 at 6:12 PM, David Pollak < >>>> > > feeder.of.the.be...@gmail.com> wrote: >>>> > > >>>> > >> We'll deprecate the Date/Calendar stuff. I'm not sure we're going >>>> to >>>> > >> remove it any time soon (in the next year +). Personally, I don't >>>> see a >>>> > >> compelling reason to remove it from the code base (there may be >>>> people >>>> > who >>>> > >> would prefer not to use JodaTime). So, we might need better names >>>> than >>>> > >> jtNow, etc., but I'm not expecting to make "now" return a DateTime. >>>> > >> >>>> > >> >>>> > >> On Mon, Oct 19, 2009 at 5:03 PM, Derek Chen-Becker < >>>> > dchenbec...@gmail.com >>>> > >> > wrote: >>>> > >> >>>> > >>> Well, yes. I actually had similar code working, but what I was >>>> trying >>>> > to >>>> > >>> avoid is a situation where someone has code like: >>>> > >>> >>>> > >>> import Helpers._ >>>> > >>> .. >>>> > >>> >>>> > >>> val someTime = now + ( 3 minutes) >>>> > >>> >>>> > >>> and then to use this code with JodaTime they need to change it to >>>> > >>> >>>> > >>> val someTime = jtNow + (3 minutes) >>>> > >>> >>>> > >>> and then when we remove the current java.util impl they would have >>>> to >>>> > >>> switch back to the original code. Also, because of all of these >>>> issues >>>> > I >>>> > >>> haven't fully worked out things like JodaSpanBuilder, which should >>>> > really be >>>> > >>> using JodaTime's Periods instead of Durations since those take >>>> into >>>> > account >>>> > >>> things like daylight savings time and leap years (none of the >>>> current >>>> > code >>>> > >>> does that right now, java.util or JodaTime). If this is the best >>>> > compromise >>>> > >>> we can come up with I'll just make a new branch to finish working >>>> on >>>> > the >>>> > >>> JodaTime side of things. >>>> > >>> >>>> > >>> Thanks, >>>> > >>> >>>> > >>> Derek >>>> > >>> >>>> > >>> >>>> > >>> On Mon, Oct 19, 2009 at 4:57 PM, David Pollak < >>>> > >>> feeder.of.the.be...@gmail.com> wrote: >>>> > >>> >>>> > >>>> Please take a look at >>>> > >>>> >>>> > >>>> http://github.com/dpp/liftweb/commit/8b5ad6d7fac428886a74356150b332443e4443e7 >>>> > >>>> >>>> > >>>> I think I've got JodaTime and the existing code playing nicely >>>> > together. >>>> > >>>> >>>> > >>>> >>>> > >>>> On Mon, Oct 19, 2009 at 2:23 PM, Derek Chen-Becker < >>>> > >>>> dchenbec...@gmail.com> wrote: >>>> > >>>> >>>> > >>>>> wip-dcb-issue-89-jodatime. I just checked in everything I have, >>>> which >>>> > >>>>> means that branch currently doesn't compile. Maybe I'm >>>> overthinking >>>> > it, but >>>> > >>>>> TimeHelpers does define some nice convenience methods and my >>>> goal is >>>> > to >>>> > >>>>> replicate and extend that with Joda Time. >>>> > >>>>> >>>> > >>>>> Derek >>>> > >>>>> >>>> > >>>>> >>>> > >>>>> On Mon, Oct 19, 2009 at 3:13 PM, David Pollak < >>>> > >>>>> feeder.of.the.be...@gmail.com> wrote: >>>> > >>>>> >>>> > >>>>>> What branch is the code on? >>>> > >>>>>> >>>> > >>>>>> >>>> > >>>>>> On Mon, Oct 19, 2009 at 1:25 PM, Derek Chen-Becker < >>>> > >>>>>> dchenbec...@gmail.com> wrote: >>>> > >>>>>> >>>> > >>>>>>> I spoke too soon. If I try to do masking imports, I get errors >>>> > about >>>> > >>>>>>> ambiguous defs: >>>> > >>>>>>> >>>> > >>>>>>> [WARNING] >>>> > >>>>>>> >>>> > >>>> /home/software/liftweb/lift-util/src/test/scala/net/liftweb/util/JodaTimeHelpersSpec.scala:71: >>>> > >>>>>>> error: reference to now is ambiguous; >>>> > >>>>>>> [WARNING] it is imported twice in the same scope by >>>> > >>>>>>> [WARNING] import _root_.net.liftweb.util.JodaTimeHelpers._ >>>> > >>>>>>> [WARNING] and import _root_.net.liftweb.util.TimeHelpers._ >>>> > >>>>>>> [WARNING] 3.seconds.later.millis must beCloseTo((now + >>>> > >>>>>>> 3.seconds).millis, 100L) >>>> > >>>>>>> >>>> > >>>>>>> Since TimeHelpers is automatically part of Helpers, and >>>> "import >>>> > >>>>>>> Helpers._" is a pretty common import, it's going to require >>>> some >>>> > other >>>> > >>>>>>> mechanism to get the proper import into scope. Any solution >>>> which >>>> > results in >>>> > >>>>>>> "now" returning a java.util.Date essentially defeats the >>>> purpose of >>>> > moving >>>> > >>>>>>> to Joda Time, since you can just do >>>> > >>>>>>> >>>> > >>>>>>> /** transforms a java.util.Date to a org.joda.time.DateTime >>>> */ >>>> > >>>>>>> implicit def dateToDateTime(in : Date) : DateTime = new >>>> > >>>>>>> DateTime(in) >>>> > >>>>>>> >>>> > >>>>>>> /** transforms an org.joda.time.DateTime to a java.util.Date >>>> */ >>>> > >>>>>>> implicit def dateTimeToDate(in : DateTime) : Date = >>>> in.toDate >>>> > >>>>>>> >>>> > >>>>>>> in your code and get most of it for free. What I'm trying to >>>> > achieve >>>> > >>>>>>> here is a refactoring of Lift's time formatting and processing >>>> API >>>> > that >>>> > >>>>>>> lends itself to clean usage of JodaTime. >>>> > >>>>>>> >>>> > >>>>>>> Derek >>>> > >>>>>>> >>>> > >>>>>>> >>>> > >>>>>>> On Mon, Oct 19, 2009 at 2:01 PM, Derek Chen-Becker < >>>> > >>>>>>> dchenbec...@gmail.com> wrote: >>>> > >>>>>>> >>>> > >>>>>>>> The major issue is the time DSL. Everything else works fine >>>> by >>>> > >>>>>>>> masking the definitions with a later import, e.g. >>>> > >>>>>>>> >>>> > >>>>>>>> import Helpers._ >>>> > >>>>>>>> import JodaTimeHelpers._ >>>> > >>>>>>>> >>>> > >>>>>>>> The problem is that implicit defs (views) don't mask, so I >>>> can't >>>> > >>>>>>>> override or mask it so that "3 seconds" returns a Joda Time >>>> > Duration, which >>>> > >>>>>>>> is a cleaner way of representing time durations in >>>> milliseconds. >>>> > In order to >>>> > >>>>>>>> get a better discussion going, I'll go ahead and comment out >>>> my >>>> > refactored >>>> > >>>>>>>> DSL and put up the code that does work this afternoon and we >>>> can >>>> > discuss it >>>> > >>>>>>>> from there. >>>> > >>>>>>>> >>>> > >>>>>>>> Derek >>>> > >>>>>>>> >>>> > >>>>>>>> >>>> > >>>>>>>> On Mon, Oct 19, 2009 at 1:37 PM, David Pollak < >>>> > >>>>>>>> feeder.of.the.be...@gmail.com> wrote: >>>> > >>>>>>>> >>>> > >>>>>>>>> I've just looked at the code. >>>> > >>>>>>>>> >>>> > >>>>>>>>> There's nothing that's going to be incompatible moving to >>>> > >>>>>>>>> JodaTime. Sure, the month method will have a different >>>> result if >>>> > you pass >>>> > >>>>>>>>> it a JodaTime instance than in you pass it a Date instance. >>>> If >>>> > this is >>>> > >>>>>>>>> documented, then it's not a problem. >>>> > >>>>>>>>> >>>> > >>>>>>>>> Existing methods don't get changed. For methods that return >>>> a >>>> > Date >>>> > >>>>>>>>> or Calendar, there'll be a parallel method (e.g., jNow or >>>> > somesuch) that >>>> > >>>>>>>>> returns a JodaTime instance. >>>> > >>>>>>>>> >>>> > >>>>>>>>> Existing methods that take a Date or Calendar, there'll be >>>> > parallel >>>> > >>>>>>>>> methods that take a JodaTime instance. >>>> > >>>>>>>>> >>>> > >>>>>>>>> Where does this seem unworkable? >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> On Mon, Oct 19, 2009 at 12:26 PM, Jeppe Nejsum Madsen < >>>> > >>>>>>>>> je...@ingolfs.dk> wrote: >>>> > >>>>>>>>> >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> Derek Chen-Becker <dchenbec...@gmail.com> writes: >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> > That's pretty much my take. The whole Java >>>> > >>>>>>>>>> Calendar/Date/Timezone impl is >>>> > >>>>>>>>>> > poorly designed, hence Joda Time. >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> > Now I've run into another wall, this time with the >>>> > >>>>>>>>>> TimeSpanBuilder. I can't >>>> > >>>>>>>>>> > mask the implicits from long/int to TimeSpanBuilder, so I >>>> > can't >>>> > >>>>>>>>>> define the >>>> > >>>>>>>>>> > same DSL for things like "3 seconds", etc. I tried to >>>> provide >>>> > an >>>> > >>>>>>>>>> implicit >>>> > >>>>>>>>>> > conversion from TimeSpan to JodaSpan but that won't cover >>>> all >>>> > of >>>> > >>>>>>>>>> the cases >>>> > >>>>>>>>>> > and this is feeling increasingly ugly as I add more and >>>> more >>>> > >>>>>>>>>> layers and >>>> > >>>>>>>>>> > implicit defs to work around the old API. >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> I haven't looked very closely at the current code, so don't >>>> > really >>>> > >>>>>>>>>> know >>>> > >>>>>>>>>> what the issues are..... but this won't stop me from >>>> commenting. >>>> > >>>>>>>>>> I'm >>>> > >>>>>>>>>> really looking forward to get rid of Calendar and >>>> friends.... >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> > At this point, the only way that this may work is if I >>>> create >>>> > >>>>>>>>>> and entirely >>>> > >>>>>>>>>> > new JodaHelpers object that extends all of the Helpers >>>> traits, >>>> > >>>>>>>>>> replacing >>>> > >>>>>>>>>> > TimeHelpers with my new JodaTimeHelpers trait. This may >>>> be the >>>> > >>>>>>>>>> path of least >>>> > >>>>>>>>>> > pain, but it means two cycles of deprecation: one to >>>> deprecate >>>> > >>>>>>>>>> the current >>>> > >>>>>>>>>> > TimeHelpers in favor of JodaTimeHelpers, and the second >>>> to >>>> > >>>>>>>>>> deprecate >>>> > >>>>>>>>>> > JodaTimeHelpers back to a refactored/reworked >>>> TimeHelpers. >>>> > Does >>>> > >>>>>>>>>> this sound >>>> > >>>>>>>>>> > like a reasonable approach, or is this getting too crazy? >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> Depending on the cycle-length, this doesn't sound too bad. >>>> But >>>> > if >>>> > >>>>>>>>>> it is >>>> > >>>>>>>>>> 1.3 before we get pure JodaTime, this sounds excessive. >>>> Lift 1.1 >>>> > >>>>>>>>>> already >>>> > >>>>>>>>>> has numerous breaking changes so if a cycle is just a >>>> milestone >>>> > >>>>>>>>>> release >>>> > >>>>>>>>>> then I think it's fine. >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> > The other option I thought of is to make a new branch >>>> just >>>> > for >>>> > >>>>>>>>>> this and >>>> > >>>>>>>>>> > just track master with the branch. The big downside is >>>> that >>>> > this >>>> > >>>>>>>>>> doubles the >>>> > >>>>>>>>>> > release work to have a Joda Lift and non-Joda Lift. >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> Doesn't sound like a viable approach. We'll be carrying >>>> this >>>> > >>>>>>>>>> baggage >>>> > >>>>>>>>>> forever (unless this was only a temp solution?) >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> /Jeppe >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> >>>> > >>>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> -- >>>> > >>>>>>>>> Lift, the simply functional web framework >>>> http://liftweb.net >>>> > >>>>>>>>> Beginning Scala http://www.apress.com/book/view/1430219890 >>>> > >>>>>>>>> Follow me: http://twitter.com/dpp >>>> > >>>>>>>>> Surf the harmonics >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>>>> >>>> > >>>>>>> >>>> > >>>>>>> >>>> > >>>>>>> >>>> > >>>>>>> >>>> > >>>>>> >>>> > >>>>>> >>>> > >>>>>> -- >>>> > >>>>>> Lift, the simply functional web framework http://liftweb.net >>>> > >>>>>> Beginning Scala http://www.apress.com/book/view/1430219890 >>>> > >>>>>> Follow me: http://twitter.com/dpp >>>> > >>>>>> Surf the harmonics >>>> > >>>>>> >>>> > >>>>>> >>>> > >>>>>> >>>> > >>>>> >>>> > >>>>> >>>> > >>>>> >>>> > >>>> >>>> > >>>> >>>> > >>>> -- >>>> > >>>> Lift, the simply functional web framework http://liftweb.net >>>> > >>>> Beginning Scala http://www.apress.com/book/view/1430219890 >>>> > >>>> Follow me: http://twitter.com/dpp >>>> > >>>> Surf the harmonics >>>> > >>>> >>>> > >>>> >>>> > >>>> >>>> > >>> >>>> > >>> >>>> > >>> >>>> > >> >>>> > >> >>>> > >> -- >>>> > >> Lift, the simply functional web framework http://liftweb.net >>>> > >> Beginning Scala http://www.apress.com/book/view/1430219890 >>>> > >> Follow me: http://twitter.com/dpp >>>> > >> Surf the harmonics >>>> > >> >>>> > >> >> >>>> > >> >>>> > > >>>> > >>>> > >>>> > >>>> > > >>>> > >>>> >>>> >>>> >>>> >>>> >>> >>> >>> >> >> >> -- >> Lift, the simply functional web framework http://liftweb.net >> Beginning Scala http://www.apress.com/book/view/1430219890 >> Follow me: http://twitter.com/dpp >> Surf the harmonics >> >> >> > > > > -- Lift, the simply functional web framework http://liftweb.net Beginning Scala http://www.apress.com/book/view/1430219890 Follow me: http://twitter.com/dpp Surf the harmonics --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Lift" group. To post to this group, send email to liftweb@googlegroups.com To unsubscribe from this group, send email to liftweb+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/liftweb?hl=en -~----------~----~----~----~------~----~------~--~---
