-
Notifications
You must be signed in to change notification settings - Fork 386
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #6004: Fix coding standards. #4370
Conversation
Related to: backdrop/backdrop-issues#6004 |
Tugboat has finished building a preview for this pull request! Website: https://pr4370-eysfgqgyadntnnd9yonnmivssgqguu7v.tugboatqa.com/ This preview will automatically expire on the 28th of April, 2023. |
* The input data array from a request to be sanitized. | ||
* @param string[] $allowlist | ||
* @param array $allowlist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe string[]
is fine here and elsewhere. array
would be less precise. From what I understand string[]
means an array of strings.
* (optional) Whether to return a list of options instead of objects. This | ||
* list is suitable for use in a select list #options array. This list is | ||
* ordered by weight and then language name. | ||
* @param bool $native_names | ||
* (optional) Whether the option_list labels should use the native name | ||
* instead of the English language name. Only applies if $option_list is TRUE. | ||
* | ||
* @return (stdClass|string)[] | ||
* @return array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be instead: (object|string)[]
.
I found this reference: https://docs.phpdoc.org/3.0/guide/references/phpdoc/types.html
* The HTTP header name, or the special 'Status' header name. | ||
* @param $value | ||
* @param string|false $value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about having false
or true
in @param
. It might be allowed by our standards but I think I read somewhere how it can be set for the return value: @return false
but not for @param
. But now I can't find it or maybe it was just my impression.
Maybe https://docs.phpdoc.org/3.0/guide/guides/types.html#supported-types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's okay. I still haven't ready anything either way so I assume it might be ok.
https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md#53-tags
For backdrop/backdrop-issues#6004