1. <?php
* @param int $account
*   The account ID to check.
?>

    $account is not an int.
    /core/modules/tracker/tracker.module -> _tracker_myrecent_access(AccountInterface $account)

  2. @param detail is missing
    /core/modules/tracker/tracker.pages.inc -> tracker_page($account = NULL)
CommentFileSizeAuthor
#12 interdiff-2608890-9-12.txt498 bytesashhishhh
#12 improve_doc_tracker-2608890-12.patch1.06 KBashhishhh
#9 interdiff-2608890-2-9.txt976 bytesashhishhh
#9 improve_doc_tracker-2608890-9.patch1.07 KBashhishhh
#6 interdiff-2608890-2-6.txt496 bytesashhishhh
#6 improve_doc_tracker-2608890-6.patch1.05 KBashhishhh
#2 imporove_doc_tracker-2608890-2.patch962 bytesashhishhh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ashhishhh created an issue. See original summary.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
imporove_doc_tracker-2608890-2.patch962 bytes
ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Issue summary: View changes
ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Active » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks for the patch! Sorry for delay in review -- I've been on vacation.

This patch needs a bit of work, I think:

  1. +++ b/core/modules/tracker/tracker.module
@@ -135,7 +135,7 @@ function tracker_cron() {
- * @param int $account
+ * @param \Drupal\Core\Session\AccountInterface $account
  *   The account ID to check.

    The docs here still say it is an account ID. That is incorrect. 

  2. +++ b/core/modules/tracker/tracker.pages.inc
@@ -14,6 +14,9 @@
+ * @param object $account
+ *   (optional) The user account to track.

    It seems unlikely, in 8.x, that what is passed in here is a generic "object".

    If you go to
    https://api.drupal.org/api/drupal/core!modules!tracker!tracker.pages.inc...
    and look at the 3 calls to this function, the two that pass something in here are passing in a UserInterface object. So we should document it that way I think.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
improve_doc_tracker-2608890-6.patch1.05 KB
interdiff-2608890-2-6.txt496 bytes

@jhodgdon Thanks.
Giving a new patch.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Needs work » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks, but according to the interdiff, only point 1 from review comment #5 was fixed. Please also fix point 2.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
improve_doc_tracker-2608890-9.patch1.07 KB
interdiff-2608890-2-9.txt976 bytes

@jhodgdon,
Yes, I missed point 2.
Now added.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Needs work » Needs review
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

Thanks! However, it doesn't look to me as though in tracker.module, the $account parameter to _tracker_myrecent_access() is optional. There is no default.

In the other function, in tracker.pages.inc, that one is optional.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
improve_doc_tracker-2608890-12.patch1.06 KB
interdiff-2608890-9-12.txt498 bytes

oops, I missed that.
Here is the correction.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Needs work » Needs review

  • jhodgdon committed 136b1d4 on 8.0.x 
    Issue #2608890 by ashhishhh: Improve comment standards for tracker...

  • jhodgdon committed c172aec on 
    Issue #2608890 by ashhishhh: Improve comment standards for tracker...
jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Fixed

Thanks! Looks good to me. Committed to both 8.x branches.

Status: Fixed » Closed (fixed)

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