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

Correct cache store superclass in comment #21300

Merged
merged 1 commit into from
Aug 20, 2015
Merged

Correct cache store superclass in comment #21300

merged 1 commit into from
Aug 20, 2015

Conversation

jonahb
Copy link
Contributor

@jonahb jonahb commented Aug 19, 2015

The documentation of ActiveSupport::Cache.lookup_store refers to CacheStore, but the superclass of cache stores (MemoryStore, MemCacheStore, etc.) is Store. This commit corrects the error. Readers who want to inspect the implementation won't waste time searching for a class called CacheStore — as I just did!

@herminiotorres
Copy link
Contributor

though it may be a simple fix, I think it has been worth more complete message stating why instead of saying the obvious

@jonahb
Copy link
Contributor Author

jonahb commented Aug 19, 2015

@herminiotorres I don't quite understand. Are you asking for a more complete commit message?

@herminiotorres
Copy link
Contributor

It has no description on its message pull request!

Yes, in your commit message, you just said the obvious.

If I see the changes your commit message is redundant because this change it is simple.

It would be better if you could speak their intention on this change.

It makes sense? 💅

@jonahb
Copy link
Contributor Author

jonahb commented Aug 19, 2015

OK, @herminiotorres, I've added a description to the pull request that clarifies my intentions. Let me know if you'd like the commit message also to clarify the rationale (though IMO this does the job).

@jonahb jonahb changed the title Correct cache superclass name in comment Correct cache store superclass in comment Aug 19, 2015
senny added a commit that referenced this pull request Aug 20, 2015
Correct cache store superclass in comment [ci skip]
@senny senny merged commit 013dd75 into rails:master Aug 20, 2015
@senny
Copy link
Member

senny commented Aug 20, 2015

@jonahb good catch 💛 For further documentation patches, please include [ci skip] in the commit message. This will prevent our CI from running needlessly.

senny added a commit that referenced this pull request Aug 20, 2015
Correct cache store superclass in comment [ci skip]
@jonahb
Copy link
Contributor Author

jonahb commented Aug 20, 2015

@senny will do, thanks

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.

None yet

3 participants