in File : core/modules/user/src/PrivateTempStore.php , Wrong argument documented as @param in the comment doc of the constructor in Class PrivateTempStore

Here is the changes, 

@@ -79,8 +79,10 @@ class PrivateTempStore {
    *   of key/value pairs.
    * @param \Drupal\Core\Lock\LockBackendInterface $lockBackend
    *   The lock object used for this data.
-   * @param mixed $owner
-   *   The owner key to store along with the data (e.g. a user or session ID).
+   * @param \Drupal\Core\Session\AccountProxyInterface $current_user
+   *   The current user account.
+   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
+   *   The Request.
    * @param int $expire
    *   The time to live for items, in seconds.
CommentFileSizeAuthor
#19 interdiff-4-19.txt1.33 KBrang501
#19 wrong-arg-comment-doc-2606304-19.patch803 bytesrang501
#15 wrong-arg-comment-doc-2606304-15.patch476 bytessnehi
#15 interdiff.txt476 bytessnehi
#9 interdiff.txt476 bytespriya.chat
#9 wrong-arg-comment-doc-2606304-9.patch476 bytespriya.chat
#4 interdiff.txt573 bytessnehi
#4 wrong-arg-comment-doc-2606304-4.patch796 bytessnehi
#2 wrong-arg-comment-doc-1.patch796 bytesrakesh.gectcr

Comments

rakesh.gectcr created an issue. See original summary.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented
StatusFileSize
new wrong-arg-comment-doc-1.patch796 bytes
jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +rc eligible 
+++ b/core/modules/user/src/PrivateTempStore.php
@@ -79,8 +79,10 @@ class PrivateTempStore {
+   *   The Request.

I don't think Request should be capitalized here.

The rest looks good to me, thanks!

snehi’s picture

snehi commented
Assigned: rakesh.gectcr » snehi
Status: Needs work » Needs review
StatusFileSize
new wrong-arg-comment-doc-2606304-4.patch796 bytes
new interdiff.txt573 bytes

@rakesh.gectcr,
Thanks for the issue and patch.
Please add issue number in the patch file from the next time.
Right now i have changed Request to request as suggested and attaching patch with interdiff.

rakesh.gectcr’s picture

rakesh.gectcr
he/him
Primary language English
Location Manchester
commented

@snehi,

Please work on the unassigned issues from the queue. It was assigned to me. Hope you wont be breaking the community work flow again.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Needs work
Issue tags: -rc eligible

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

+++ b/core/modules/user/src/PrivateTempStore.php
@@ -79,8 +79,10 @@ class PrivateTempStore {
+   * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack
+   *   The request.

This doesn't look right... its class is RequestStack, which is ... not the request, is it? I really don't know what a "request stack" is, but it doesn't seem like it's a "request".

snehi’s picture

snehi commented
Assigned: snehi » Unassigned
priya.chat’s picture

priya.chat commented
Assigned: Unassigned » priya.chat
priya.chat’s picture

priya.chat commented
Status: Needs work » Needs review
StatusFileSize
new wrong-arg-comment-doc-2606304-9.patch476 bytes
new interdiff.txt476 bytes

Hello,
I have added the comments for 'Request Stack' in the patch. please review this.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! That looks reasonable to me.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Hm. Sorry about my last review, which was incorrect. This latest patch is not covering the stated issue at all. Please go back to the patch in #4 and fix that instead.

priya.chat’s picture

priya.chat commented
Assigned: priya.chat » Unassigned
snehi’s picture

snehi commented

@jhodgdon can you please give hint that what should be replaced by

The request

Thanks in advance.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

probably request stack? I don't know but it isn't the request.

snehi’s picture

snehi commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt476 bytes
new wrong-arg-comment-doc-2606304-15.patch476 bytes

Status: Needs review » Needs work

The last submitted patch, 15: wrong-arg-comment-doc-2606304-15.patch, failed testing.

snehi’s picture

snehi commented

The request stack is already mentioned in latest pull. So nothing in diff with 8.0.x branch uploading just the interdiff.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Latest patch does not apply.

rang501’s picture

rang501 commented
Status: Needs work » Needs review
StatusFileSize
new wrong-arg-comment-doc-2606304-19.patch803 bytes
new interdiff-4-19.txt1.33 KB

Hi! It seems that latest patch are missing some stuff. I tried to follow the comments and put together a better (IMO) patch based on #4.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

Thanks! Much better.

snehi’s picture

snehi commented 
+++ a/core/modules/user/src/PrivateTempStore.php
@@ -79,10 +79,8 @@
+   *   The owner key to store along with the data (e.g. a user or session ID).

Are we sure to use along with data.
Hows it
key to store with data.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

#21 looks like an unrelated issue. If you think it is a problem, please file a separate issue. This issue is only about some @param docs that have the incorrect variable names in this class.

  • catch committed 436ac08 on 8.1.x 
    Issue #2606304 by snehi, priya.chat, rang501, rakesh.gectcr: Wrong @...

  • catch committed 9c4c596 on 8.0.x 
    Issue #2606304 by snehi, priya.chat, rang501, rakesh.gectcr: Wrong @...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!

Status: Fixed » Closed (fixed)

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