-
Notifications
You must be signed in to change notification settings - Fork 38
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 Cache specification #35
Conversation
Looks good. 👍 Let's finish #34 before we merge this one. |
Cool, sounds good, I'll also add factory method for cache :) |
hehe great =) Because that is what our contribution file says =) |
Hehe, nice :) I'm on a train now so wifi is not good, so in worst case I'll update it when I'll be back in London :) |
@Nyholm updated :) |
*/ | ||
public function modifyQuery(AbstractQuery $query) | ||
{ | ||
$query->setResultCacheLifetime($this->cacheLifetime); |
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.
Did you leave out $this->parent->modifyQuery($query)
intentionally?
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.
@Nyholm your question led me to some conclusions, will open a new issue.
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.
which is?
please @cakper could you rebase? |
This is not blocked anymore (since merge of #56) |
all road blocks were taken off, ping. I think adding adding cache is complicating unnecessarily the library for now but ok. |
Blocked by #39
@Nyholm I've prepared cache specification, basic one so far, but I think we may need to add options for Cache Profiles, Drivers etc.