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

TimeSpan represents duration in milliseconds. #1668

Closed
wants to merge 5 commits into from

Conversation

arkadius
Copy link
Contributor

Mailing list discussion: https://groups.google.com/forum/#!topic/liftweb/CgZFVwMEiqU

Before the change TimeSpan has many responsibilities - in equality behave as a joda Period (was checking duration field equality), can represent a Date (using toDate/toDateTime operations) or be used as a simple container for milliseconds. Now is responsible only for operations on milliseconds. Problematic methods have been deprecated (.ago/.later have been moved to implicit PeriodExtension). For backward compatibility there is introduced implicit conversion from Period to TimeSpan.

Period inside TimeSpan is holded now only for puropose of to-period conversion (for months/years builder). There has been used view bounds for purpose of nested conversions usage:
1.second.before
1 -> TimeSpan -> Period

This is WIP branch. Please send me feedback if my english has failed in deprecation descriptions :-)

Before the change TimeSpan has many responsibilities - in equality behave as a joda Period (was checking duration field equality), can represent a Date (using toDate/toDateTime operations) or be used as a simple container for milliseconds. Now is responsible only for operations on milliseconds. Problematic methods have been deprecated (.ago/.later have been moved to implicit PeriodExtension). For backward compatibility there is introduced implicit conversion from Period to TimeSpan.
def toMillis = millis

/**
* @return amount of milliseconds in duration
* @throws UnsupportedOperationException if was created by deprecated months/years builder
Copy link
Member

Choose a reason for hiding this comment

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

This is documenting existing behavior rather than a new exception, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yest, It's documenting existing behavior

@fmpwizard fmpwizard added this to the 3.0-M3 milestone Jan 14, 2015
def week = weeks
def months = new TimeSpan(Right((new Period().plusMonths(len.toInt))))
@deprecated("TimeSpan will not support operations on non-millis periods in future")
Copy link
Member

Choose a reason for hiding this comment

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

I believe @deprecated takes two parameters, one is the text you have here and the other is the version, so 3.0

Copy link
Member

Choose a reason for hiding this comment

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

Let's use 3.0.0. I think it's worth going full semver for the 3 series, because why not :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done it

@arkadius
Copy link
Contributor Author

Let me know when you fill that all looks fine. Then I will prepare branch with squashed changes.

@Shadowfiend
Copy link
Member

Sorry, I haven't had a chance to give this a second, deeper look. I'll try and do that tomorrow so we can roll out the M3 build! For time's sake, if everything looks reasonably good I'll probably make any tweaks to history and verbiage and merge it in at that time.

@Shadowfiend
Copy link
Member

Okay, I want most of this to land in M3, so I undid one thing I want to discuss a little more–namely the deprecations on ConvertableToDate. I'll bring it up on the list and we can figure that out and get any related changes in for M4.

I also reworded some of the deprecation notices and listed alternatives for all of them, and added a block about the deprecation to the TimeSpan scaladocs.

I put the result in #1670; let's try to merge that, close this, and resolve the remaining couple of questions on the list!

@fmpwizard fmpwizard removed this from the 3.0-M3 milestone Jan 18, 2015
@Shadowfiend Shadowfiend added this to the 3.0-M3 milestone Jan 18, 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

Successfully merging this pull request may close these issues.

None yet

3 participants