DDEV is the official local development tool of Drupal. And like Drupal, DDEV depends on the support of the open source community.Problem/Motivation
The docs for Drupal\views\Plugin\views\cache\CachePluginBase::cacheSetMaxAge() say:
> * Determine cache expiration time.
and there is no @return documentation.
This makes it look like the function returns the expiry time value to be put into the cache.
However, looking at cacheSet() in the same class, we can see it expects a relative offset from the request time:
$expire = ($this->cacheSetMaxAge($type) === Cache::PERMANENT) ? Cache::PERMANENT : (int) $this->view->getRequest()->server->get('REQUEST_TIME') + $this->cacheSetMaxAge($type);
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 3480732-nr-bot.txt | 1.93 KB | needs-review-queue-bot |
Issue fork drupal-3480732
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedWorking on it.
Comment #4
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedHi @joachim,
I've opened the MR, Please have a look:)
Thanks!
Comment #5
joachim CreditAttribution: joachim as a volunteer commentedThanks for the MR! Though remember to change the status to Needs Review when you want it looked at.
I've left comments on the MR for changes.
Comment #7
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedComment #8
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedComment #9
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedHello @joachim, please let me know if the changes are alright.
Comment #10
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #11
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedHello @joachim, please review the changes.
Comment #12
joachim CreditAttribution: joachim at Chicken commentedLooks good, thanks!
Comment #13
quietone CreditAttribution: quietone at PreviousNext commented@debrup, thanks for staying with this!
One suggestion in the MR that needs to be considered.
Comment #14
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedHello @joachim,
'Defaults to' has been used all across the codebase in similar situations, even in methods present in various Interfaces. I think it will be beneficial to maintain the same format used as it will be easier to understand by others. Plus since we are wanting to do the job of an interface so following the common interface format will not do any harm:)
Comment #15
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedHello @joachim @quietone, Please review the changes.
Comment #16
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commentedComment #17
debrup CreditAttribution: debrup at Innoraft for Drupal India Association commented@joachim @quietone please review the changes.
Comment #18
gaurav gupta CreditAttribution: gaurav gupta at Innoraft for Drupal India Association commentedThe changes has been done as per the suggestions in #13.
Moving it to RTBC.
Thanks.
Comment #19
quietone CreditAttribution: quietone at PreviousNext commentedComment #23
quietone CreditAttribution: quietone at PreviousNext commentedCommitted and pushed to 11.x and cherry-picked to 11.1.x only, here were conflicts in the phpstan baseline for other branches.
Thanks for improving the documentation in core!