API page: https://api.drupal.org/api/drupal/core%21modules%21views%21src%21Plugin%...

Missing @param for $type and missing @return.

CommentFileSizeAuthor
#5 cacheplugin_2951163_5.patch1.03 KBnkoporec
#2 2951163-2.patch1.02 KBparvateesam

Comments

joachim created an issue. See original summary.

parvateesam’s picture

parvateesam commented
Status: Active » Needs review
StatusFileSize
new 2951163-2.patch1.02 KB

I have added @param, @return for cacheGet() and @param for cacheSet() as well.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs review » Needs work 
+   * @return bool
+   *   True if data is been taken from cache otherwise false.

I think there is grammar error, the sentence should be: True if data has been taken from the cache otherwise return false.

joachim’s picture

joachim commented

> I think there is grammar error, the sentence should be: True if data has been taken from the cache otherwise return false.

I don't think we need the extra 'return' in there, but:

1. IIRC constants should be in capitals even in docs: TRUE, not true.

2. a comma would help readability.

So:

> TRUE if data has been taken from the cache, otherwise FALSE.

nkoporec’s picture

nkoporec
Primary language English
Location Slovenia
commented
Status: Needs work » Needs review
StatusFileSize
new cacheplugin_2951163_5.patch1.03 KB

Created a new patch with fixed description.

amietpatial’s picture

amietpatial commented

@ nkoporec Patch #5 applied successfully

msankhala’s picture

msankhala commented
Title: CachePluginBase::cacheGet() doesn't document @params or @return » CachePluginBase::cacheGet()/::cacheSet() doesn't document @params or @return
msankhala’s picture

msankhala commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Novice

I have updated the issue title because this patch is actually fixing docblock for both CachePluginBase::cacheGet() and CachePluginBase::cacheSet() .

I can confirm this patch #5 applies cleanly.

  • catch committed 11f7f2a on 8.6.x 
    Issue #2951163 by nkoporec, Parvateesam, joachim: CachePluginBase::...
catch’s picture

catch
he/him
Primary language English
commented
Version: 8.6.x-dev » 8.5.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!

  • catch committed f11912e on 8.5.x 
    Issue #2951163 by nkoporec, Parvateesam, joachim: CachePluginBase::...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.