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

Fix compatibility with AS Concern #26

Merged
merged 4 commits into from
Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## [Unreleased]

### Fixed
- Fix compatibility with `ActiveSupport::Concern`. (revert [#23])

## [1.3.0] - 2020-02-10
### Added
- Allow memoization after including module with Memery. ([@AlexWayfer]) [#23]
Expand Down
13 changes: 13 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,17 @@ PATH
GEM
remote: https://rubygems.org/
specs:
activesupport (5.2.4.2)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (>= 0.7, < 2)
minitest (~> 5.1)
tzinfo (~> 1.1)
ast (2.4.0)
benchmark-ips (2.7.2)
benchmark-memory (0.1.2)
memory_profiler (~> 0.9)
coderay (1.1.2)
concurrent-ruby (1.1.6)
coveralls (0.8.23)
json (>= 1.8, < 3)
simplecov (~> 0.16.1)
Expand All @@ -20,10 +26,13 @@ GEM
tins (~> 1.6)
diff-lcs (1.3)
docile (1.3.2)
i18n (1.8.2)
concurrent-ruby (~> 1.0)
jaro_winkler (1.5.4)
json (2.3.0)
memory_profiler (0.9.14)
method_source (0.9.2)
minitest (5.14.0)
parallel (1.19.1)
parser (2.7.0.2)
ast (~> 2.4.0)
Expand Down Expand Up @@ -76,14 +85,18 @@ GEM
term-ansicolor (1.7.1)
tins (~> 1.0)
thor (1.0.1)
thread_safe (0.3.6)
tins (1.24.0)
sync
tzinfo (1.2.6)
thread_safe (~> 0.1)
unicode-display_width (1.6.1)

PLATFORMS
ruby

DEPENDENCIES
activesupport (~> 5.0)
benchmark-ips
benchmark-memory
bundler
Expand Down
19 changes: 6 additions & 13 deletions lib/memery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,15 @@

module Memery
class << self
def monotonic_clock
Process.clock_gettime(Process::CLOCK_MONOTONIC)
end
end

module ModuleMethods
def included(base)
base.extend(ClassMethods)
base.include(InstanceMethods)
base.extend ModuleMethods if base.instance_of?(Module)
end
end

extend ModuleMethods
def monotonic_clock
Process.clock_gettime(Process::CLOCK_MONOTONIC)
end
end

module ClassMethods
def memoize(method_name, condition: nil, ttl: nil)
Expand All @@ -32,16 +27,14 @@ def memoized?(method_name)
return false unless defined?(@_memery_module)

@_memery_module.method_defined?(method_name) ||
@_memery_module.private_method_defined?(method_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like extra (non-related) change.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yep, but I'm usually OK with some minimal cosmetic changes :)

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, but let's add the RuboCop to the CI of this repository and check if it's OK.

Can we switch from Travis CI? I have no permissions to do this, and I suggest Cirrus CI or Circle CI (a config of the first one will be shorten).

Copy link
Owner Author

Choose a reason for hiding this comment

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

There actually is rubocop in the CI. It gets called in the bundle exec rake command.

And btw what's the problem with Travis?

Copy link
Contributor

Choose a reason for hiding this comment

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

There actually is rubocop in the CI. It gets called in the bundle exec rake command.

Oh, OK, then it's strange that both approaches (with indentation and without) are acceptable.

And btw what's the problem with Travis?

I had many issues with environment, but if it satisfies you — never mind. Also, it uses the deprecated method of integration with GitHub, as I know.

Also: …

image

@_memery_module.private_method_defined?(method_name)
end

private

def prepend_memery_module!
return if defined?(@_memery_module)
@_memery_module = Module.new do
extend MemoizationModule
end
@_memery_module = Module.new { extend MemoizationModule }
prepend @_memery_module
end

Expand Down
1 change: 1 addition & 0 deletions memery.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Gem::Specification.new do |spec|

spec.add_runtime_dependency "ruby2_keywords", "~> 0.0.2"

spec.add_development_dependency "activesupport", "~> 5.0"
spec.add_development_dependency "benchmark-ips"
spec.add_development_dependency "benchmark-memory"
spec.add_development_dependency "bundler"
Expand Down
29 changes: 19 additions & 10 deletions spec/memery_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,6 @@ def not_memoized; end

class C
include M

memoize def m_class
CALLS << __method__
__method__
end
end

class D
Expand Down Expand Up @@ -250,14 +245,28 @@ def self.macro(name)
expect(values).to eq([:m, :m, :m])
expect(CALLS).to eq([:m])
end
end

context "memoization in class" do
specify do
values = [c.m_class, c.m_class, c.m_class]
expect(values).to eq([:m_class, :m_class, :m_class])
expect(CALLS).to eq([:m_class])
context "module with self.included method defined" do
subject(:c) { C.new }

before { C.include(some_mixin) }

let(:some_mixin) do
Module.new do
extend ActiveSupport::Concern
include Memery

included do
attr_accessor :a
end
end
end

it "doesn't override existing method" do
c.a = 15
expect(c.a).to eq(15)
end
end

context "class method with args" do
Expand Down
1 change: 1 addition & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
SimpleCov.start

require "memery"
require "active_support/concern"

RSpec.configure do |config|
# Enable flags like --only-failures and --next-failure
Expand Down