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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Duration#before and #after as aliases for #ago and #since #27721

Closed
wants to merge 2 commits into from
Closed

Add Duration#before and #after as aliases for #ago and #since #27721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28017b4
Add Duration#before and #after as aliases for #ago and #since
Widdershin Feb 22, 2017
7f208fe
Merge branch 'master' into master
Widdershin Feb 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions activesupport/CHANGELOG.md
View file
Open in desktop
@@ -1,3 +1,19 @@
* Add `ActiveSupport::Duration#before` and `#after` as aliases for `#until` and `#since`

These read more like English and require less mental gymnastics to read and write.

Before:

2.weeks.since(customer_start_date)
5.days.until(today)

After:

2.weeks.after(customer_start_date)
5.days.before(today)

*Nick Johnstone*

* Cache `ActiveSupport::TimeWithZone#to_datetime` before freezing.

*Adam Rice*
Expand Down
2 changes: 2 additions & 0 deletions activesupport/lib/active_support/duration.rb
View file
Open in desktop
Expand Up @@ -180,13 +180,15 @@ def since(time = ::Time.current)
sum(1, time)
end
alias :from_now :since
alias :after :since

# Calculates a new Time or Date that is as far in the past
# as this Duration represents.
def ago(time = ::Time.current)
sum(-1, time)
end
alias :until :ago
alias :before :ago
Copy link
Member

Choose a reason for hiding this comment

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

Still feels weird to alias these鈥攁nd leave ago's required argument as optional鈥攔ather than define new methods. Not necessary for this PR, but worth considering

# Calculates this duration in the past relative to +date_or_time+.
def before(date_or_time)
  sum(-1, date_or_time)
end
alias :until :before

def ago
  before ::Time.current
end

That'd break people using the odd 2.days.ago customer_start_date and 5.weeks.since formulations, though, so it'd need deprecation 馃槓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to make this change if it's desired, but as you say, perhaps in a different PR. What is the process for deprecating something like that?

Copy link
Member

Choose a reason for hiding this comment

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

The process is to allow both old and new method signatures and give a deprecation warning if we see the old one.

# Calculates this duration in the past relative to +date_or_time+.
def before(date_or_time)
  sum(-1, date_or_time)
end
alias :until :before

def ago(deprecated_date_or_time = nil)
  if deprecated_date_or_time
    ActiveSupport::Deprecation.warn "Passing a date_or_time argument to #ago is deprecated and will be removed in Rails 5.2. Use #before(date_or_time) instead."
  end
  before deprecated_date_or_time || ::Time.current
end


def inspect #:nodoc:
parts.
Expand Down
13 changes: 13 additions & 0 deletions activesupport/test/core_ext/duration_test.rb
View file
Open in desktop
Expand Up @@ -179,6 +179,19 @@ def test_since_and_ago_anchored_to_time_zone_now_when_time_zone_is_set
Time.zone = nil
end

def test_before_and_afer
t = Time.local(2000)
assert_equal t + 1, 1.second.after(t)
assert_equal t - 1, 1.second.before(t)
end

def test_before_and_after_without_argument
now = Time.now
assert 1.second.before >= now + 1
now = Time.now
assert 1.second.after >= now - 1
end

def test_adding_hours_across_dst_boundary
with_env_tz "CET" do
assert_equal Time.local(2009, 3, 29, 0, 0, 0) + 24.hours, Time.local(2009, 3, 30, 1, 0, 0)
Expand Down