Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Use `Time.current` in AS::CoreExtensions::Numeric::Time to maintain consistency with AS::Duration

#5607

Hi,

After I was investigate with #4076 ago (ActiveSupport::CoreExtensions::Numeric::Time) does not respect current t..., I found out that ago and since in ActiveSupport::CoreExtensions::Numeric::Time was actually using Time.now when user doesn't specify the time. However, I found out in the old commit (http://github.com/rails/rails/commit/32b82e4c#L2L54) that Geoff Buesing has switched to use Time.current to use the time zone specified in Time.default_zone instead.

So, I have made the patches to change those two lines to use Time.current instead of Time.now to make it consistence. Please review and apply them for me.

Thank you.

Reported by Prem Sichanugrist (sikachu) · September 10th, 2010 @ 04:56 PM

State: resolved
Milestone: 2.3.10
Assigned to: Geoff Buesing Geoff Buesing
Importance: Low

Activity

  1. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)
    • Assigned user changed from José Valim to Santiago Pastorino

    Santiago, could you please apply this patch for me?

    Thank you :D

    September 11th, 2010 @ 09:35 AM

  2. Santiago Pastorino
    Santiago Pastorino

    Prem the patch looks good, do you mind to add tests the same way Geoff did.
    Thanks.

    September 12th, 2010 @ 02:32 AM

  3. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)

    Here is the patch with tests added. Actually, I was copied the test from Geoff as it tests the same thing. I can confirm that it is failing before the applying the change :)

    Thank you

    September 12th, 2010 @ 08:19 AM

  4. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)

    Oh wait, I shouldn't say copy. I copy and 'modified' it.

    Thanks :)

    September 12th, 2010 @ 08:19 AM

  5. Repository
    Repository
    • State changed from new to committed

    (from [af6757a1ca7027b526b35d74c94a520d6bb24d7a]) Use Time.current to maintain consistency with AS::Duration

    In [32b82e4c6f5523cdf5ee78c3022c50b46e018351], the committer has switch
    methods in AS::Duration to use Time.current to return the correct
    duration based on the Time.default_zone instead of using Time.now.

    [#5607 Use `Time.current` in AS::CoreExtensions::Numeric::Time to maintain consisten... state:committed]

    Signed-off-by: Santiago Pastorino santiago@wyeworks.com
    http://github.com/rails/rails/commit/af6757a1ca7027b526b35d74c94a52...

    September 19th, 2010 @ 07:26 PM

  6. Repository
    Repository

    (from [e125bf264eeedf4251010eb2989dc8e5b8e33d86]) Use Time.current to maintain consistency with AS::Duration

    In [32b82e4c6f5523cdf5ee78c3022c50b46e018351], the committer has switch
    methods in AS::Duration to use Time.current to return the correct
    duration based on the Time.default_zone instead of using Time.now.

    [#5607 Use `Time.current` in AS::CoreExtensions::Numeric::Time to maintain consisten... state:committed]

    Signed-off-by: Santiago Pastorino santiago@wyeworks.com
    http://github.com/rails/rails/commit/e125bf264eeedf4251010eb2989dc8...

    September 19th, 2010 @ 07:47 PM

  7. Santiago Pastorino
    Santiago Pastorino
    • Milestone changed from 3.x to 2.3.10
    • State changed from committed to open
    • Assigned user changed from Santiago Pastorino to Geoff Buesing

    September 19th, 2010 @ 08:16 PM

  8. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)

    Santiago, why reopen? Is there anything else I can do?

    September 20th, 2010 @ 02:43 AM

  9. Santiago Pastorino
    Santiago Pastorino

    No it's ok i applied your patch on 3-0-stable and master. But ... you added a patch for 2-3-stable too and i didn't applied it.
    That's why i reopened for Milestone 2.3.10.

    September 20th, 2010 @ 04:39 AM

  10. Prem Sichanugrist (sikachu)
    Prem Sichanugrist (sikachu)
    • State changed from open to resolved

    I don't think this needed to be applied anymore, as our momentum is moving toward Rails 3. :)

    November 19th, 2010 @ 01:22 PM