-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
add check of argument #26524
add check of argument #26524
Conversation
r? @schneems (@rails-bot has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 makes sense. Should we add anything to this method's documentation to make it more explicit that you need to pass a block?
9376cda
to
68085e4
Compare
Thank you for pointing out! I added the document. How about that? |
@@ -361,6 +361,9 @@ def read_multi(*names) | |||
# the cache with the given keys, then that data is returned. Otherwise, | |||
# the supplied block is called for each key for which there was no data, | |||
# and the result will be written to the cache and returned. | |||
# Therefore, you need to pass a block that returns a data to be written | |||
# to the cache. If you do not want to write the cache when the cache is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of that returns a data to
, that returns the data to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I updated!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why, but github still marked this as 'Needs change' when you clearly did the thing. Merging.
68085e4
to
6fee11d
Compare
`#fetch_multi` in case did not cache hit, to write a cache using the block value. https://github.com/rails/rails/blob/master/activesupport/lib/active_support/cache.rb#L383..L384 Therefore, block is a need to pass always, I think should check first.
6fee11d
to
97544ed
Compare
Thanks! 🚀 |
Summary
#fetch_multi
in case did not cache hit, to write a cache using the block value.https://github.com/rails/rails/blob/master/activesupport/lib/active_support/cache.rb#L383..L384
Therefore, block is a need to pass always, I think should check first.