Feature/php doc #661
Feature/php doc #661
Conversation
geekwright
left a comment
geekwright
left a comment
There was a problem hiding this comment.
eeb6136 Changing the order in the @var clause doesn't seem critical. The definitions have historically been loose. As is, these clauses already accomplish the goal of allowing IDE auto-completion. We could take the changes with needed fixes as noted.
5aee59c Technically, this is a code change, but it does not change the execution at all. It could stay.
1775e95 There is no error in the foreachq. Yes it is odd to have a mismatched end tag, but that is how it has always been. It is a code execution change to swap it, and we should avoid those during a release cycle. The intended speed improvement has diminished as PHP has matures, but older versions may still show a small benefit. Since those older versions are the least tested, we should be cautious. (Both foreachq and its cousin includeq are already removed in 2.6.)
3e348ca It isn't broken. The existing syntax variations are documented to work, and are not errors or bugs. We shouldn't change code during a release cycle if it isn't an error.
| $sql = 'SELECT COUNT(*) as `count` FROM ' . $prefixedTable . ' '; | ||
| if (isset($criteria) && is_subclass_of($criteria, '\CriteriaElement')) { | ||
| /* @var $criteria \CriteriaCompo */ | ||
| /* @var \CriteriaCompo $criteria */ |
There was a problem hiding this comment.
Nothing inside libraries -- including xmf -- should be changed directly in XoopsCore25. There are several in this commit that need to be removed.
| * Retrieves config info based on the key. | ||
| * | ||
| * @param $file_name string config key (filename/section/var) | ||
| * @param string $file_name config key (filename/section/var) |
There was a problem hiding this comment.
Vendor code, in this case Smarty, should not be changed if at all possible. If absolutely required to change CODE, we should comment verbosely to explain the need for the change. Code reformatting and docblock changes should be avoided. It complicates reviewing the differences between version.
| * @param $uname | ||
| * | ||
| * @return userDN or false | ||
| * @return false|string $userDN |
There was a problem hiding this comment.
There is no possible way for this to return false.
| $noHtmlFilter = FilterInput::getInstance(); | ||
| } | ||
| $var = $noHtmlFilter->clean($var, $type); | ||
| $var = $noHtmlFilter::clean($var, $type); |
There was a problem hiding this comment.
No changes to XMF in XoopsCore25.
| static $jsLoaded; | ||
|
|
||
| $config = $this->loadConfig(__DIR__); | ||
| $config = static::loadConfig(__DIR__); |
There was a problem hiding this comment.
All the other textsanitizer extensions use parent::loadConfig().
2 participants
No description provided.