Use refinements to avoid polluting number classes globally

[asehra]

Hi @lashd.

Time increments usually come with activesupport on many projects. When a project has waitforit along with activesupport, then the methods from waitforit take precedence since activesupport only declares methods like seconds or hours if they do not already exist. Also activesupport's versions of these methods return instances of ActiveSupport::Duration and not numeric classes like Fixnum.

Active support also comes with support for methods like ago and from_now that are used commonly to represent time instances in a readable notation like 5.days.ago. When used with waitforit this starts throwing deprecation warnings for the #ago method since these are meant to be called on ActiveSupport::Duration objects and not Fixnums.
#ago notation is a very common pattern in rails and, thus, the monkeypatch in this gem leads to a ton of deprecation warning as soon as you start using Mirage in any rails project.

This PR proposes using Ruby's refinements instead. That way, the time increments can be used only in places that want to use these specific refinements. Example usage can be found at lib/waitforit.rb#L31

Downside is that it will work only on ruby 2.0 and above. Earlier versions of ruby are no longer supported by ruby maintainers, so this should be ok to use.

[lashd]

Thanks @asehra.

I don't think that the ruby version is a problem. Which project brought in the waitforit gem? I'd like to run the build using this update version before merging it.

[asehra]

Mirage
On 21 Mar 2016 10:42 a.m., "Leon Davis" notifications@github.com wrote:

Thanks @asehra https://github.com/asehra.

I don't think that the ruby version is a problem. Which project brought in
the waitforit gem? I'd like to run the build using this update version
before merging it.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1 (comment)

[lashd]

Cheers. In addition to this change. I am thinking of putting an internal implementation of this gem inside the mirage client to drop down the number of dependencies that mirage has.

I did something similar for page_magic here. What do you think?

[asehra]

Yeah.. Sounds good. It's a simple enough implementation to have it inline.
On 21 Mar 2016 12:12 p.m., "Leon Davis" notifications@github.com wrote:

Cheers. In addition to this change. I am thinking of putting an internal
implementation of this gem inside the mirage client to drop down the number
of dependencies that mirage has.

I did something similar here
https://github.com/lvl-up/page_magic/blob/master/lib/page_magic/wait_methods.rb.
What do you think?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1 (comment)