Skip to content

Conversation

@beauby
Copy link
Member

@beauby beauby commented Nov 21, 2016

Needs benchmark.
Closes #9

@id.to_s
end

def jsonapi_cache_key(options = {})

Choose a reason for hiding this comment

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

Unused method argument - options. If it's necessary, use _ or _options as an argument name to indicate that it won't be used. You can also write as jsonapi_cache_key(*) if you want the method to accept any arguments but don't care about them.

@id.to_s
end

def jsonapi_cache_key(options = {})

Choose a reason for hiding this comment

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

Unused method argument - options. If it's necessary, use _ or _options as an argument name to indicate that it won't be used. You can also write as jsonapi_cache_key(*) if you want the method to accept any arguments but don't care about them.

@include = include
@fields = fields


Choose a reason for hiding this comment

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

Extra blank line detected.


def fetch_multi(keys, &block)
keys.each_with_object({}) do |k, h|
@cache[k] = block.call(k) unless @cache.key?(k)

Choose a reason for hiding this comment

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

Use yield instead of block.call.

@@ -1,19 +1,33 @@
require 'set'

class Cache

Choose a reason for hiding this comment

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

Missing top-level class documentation comment.

@beauby beauby changed the title [WIP] Dirty PoC implem of caching. Fragment caching Sep 11, 2017
@jsonapi-rb jsonapi-rb deleted a comment from codecov-io Sep 11, 2017
@beauby beauby merged commit fd8923c into master Sep 11, 2017
@beauby beauby deleted the caching branch September 11, 2017 04:53
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.

3 participants