No doc block at all.

CommentFileSizeAuthor
#10 718962-alt.patch830 bytesjhodgdon
#9 doc-user_external_load-718962-9.patch845 bytesadd1sun
#2 doc-user_external_load-718962-2.patch847 bytespasqualle
#1 doc-user_external_load-718962-1.patch843 bytesadd1sun

Comments

add1sun’s picture

add1sun commented
Status: Active » Needs review
StatusFileSize
new doc-user_external_load-718962-1.patch843 bytes

Wow, look at me, I'm just making stuff up! But we should at least have a docblock here, eh?

pasqualle’s picture

pasqualle
Location 🇭🇺 Budapest
commented
StatusFileSize
new doc-user_external_load-718962-2.patch847 bytes

- removed the variable type from @param
- removed trailing spaces
- used descriptions from other functions (I am not sure if it is better)

Status: Needs review » Needs work

The last submitted patch, doc-user_external_load-718962-2.patch, failed testing.

pasqualle’s picture

pasqualle
Location 🇭🇺 Budapest
commented

@@ -183,6 +183,15 this is the problem with my patch. I just modified the patch file, and forgot that it has more lines than the first patch. So it needs a reroll.

add1sun’s picture

add1sun commented

When working on other patches Crell pointed out that adding the variable type is desirable now and some examples on http://drupal.org/node/1354 use that, so the variable type needs to go back in.

pasqualle’s picture

pasqualle
Location 🇭🇺 Budapest
commented

it's amazing how inconsistent Drupal core is.. I do not understand why they did not roll a patch when that handbook page was made..

or roll a patch for: http://drupal.org/project/coder_tough_love

@param and @return syntax does not indicate the data type.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

That's a new standard.

Rolling a single patch to figure out all of the @param and @return types and adding them to Drupal core would have been a HUGE HUGE HUGE task, and the patch would never have gotten reviewed.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Another comment on the patch in #2: All function headers should start with a verb in 3rd person, such as "Fetches" rather than "Fetch".

add1sun’s picture

add1sun commented
Status: Needs work » Needs review
StatusFileSize
new doc-user_external_load-718962-9.patch845 bytes

Rerolled

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
StatusFileSize
new 718962-alt.patch830 bytes

Looks good, except shouldn't we say ID rather than id?

Hmm... Also I think maybe we shouldn't call it a $user object, since $user is the global for the currently logged-in user...

How about this wording?

smyleeface’s picture

smyleeface commented
Status: Needs review » Reviewed & tested by the community

Formatting looks good and wording makes sense to me.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

w00t for docs! Committed to HEAD. :)

Status: Fixed » Closed (fixed)

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