{"action":"created","issue":{"url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/1919","labels_url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/1919/labels{/name}","comments_url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/1919/comments","events_url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/1919/events","html_url":"https://github.com/Yoast/wordpress-seo/pull/1919","id":54846520,"number":1919,"title":"Quality control round.","user":{"login":"jrfnl","id":663378,"avatar_url":"https://avatars.githubusercontent.com/u/663378?v=3","gravatar_id":"","url":"https://api.github.com/users/jrfnl","html_url":"https://github.com/jrfnl","followers_url":"https://api.github.com/users/jrfnl/followers","following_url":"https://api.github.com/users/jrfnl/following{/other_user}","gists_url":"https://api.github.com/users/jrfnl/gists{/gist_id}","starred_url":"https://api.github.com/users/jrfnl/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jrfnl/subscriptions","organizations_url":"https://api.github.com/users/jrfnl/orgs","repos_url":"https://api.github.com/users/jrfnl/repos","events_url":"https://api.github.com/users/jrfnl/events{/privacy}","received_events_url":"https://api.github.com/users/jrfnl/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":13,"created_at":"2015-01-20T05:51:27Z","updated_at":"2015-02-17T16:57:14Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/Yoast/wordpress-seo/pulls/1919","html_url":"https://github.com/Yoast/wordpress-seo/pull/1919","diff_url":"https://github.com/Yoast/wordpress-seo/pull/1919.diff","patch_url":"https://github.com/Yoast/wordpress-seo/pull/1919.patch"},"body":":warning: __This should only be merged once the missing documentation has been added by the team.__\r\n\r\n#### [PHPCS] Update basic ruleset & resulting fix code style issues\r\n* Fixed PHPStorm array key spacing regressions (again)\r\n* Fixed all `} else {` structures to have the else on a new line - @see https://github.com/Yoast/Wiki/wiki/Coding-guidelines\r\n\t- Added (wrote) a sniff for checking the else statements in all future commits and PRs.\r\n* The unit-tests are now also checked for code style\r\n\t- There is one line where two whitespace issues is found where I don't understand (at all) why the current code is like it is, which is why I didn't fix it.\r\n* Some of the sniffs in WP-Extra were/are not included in WP-VIP and should be run too\r\n\r\n#### [PHPCS] Added documentation checks to the ruleset & fixed a lot of the resulting issues (but not all)\r\n* As discussed with @omarreiss last Friday.\r\n* Checks that all classes, properties, methods and functions have documentation and that the documentation contains a minimum of information\r\n* Excluded all rules which I perceived would quickly become annoying\r\n* Excluded the \"class exposing\" (private to public) test files from all documentation checks\r\n* I did *not* exclude the test files in general, as unit tests ought to have some method documentation, at the very minimum the `@covers` tag to show what is being tested.\r\n* Fixed a lot of missing documentation and corrected a lot of outdated documentation.\r\n\r\n@omarreiss @jdevalk @andizer @Petervw @CarolineGeven Can each of you please have a look at the list of still missing documentation and fix the ones you feel even remotely responsible for ? Let me know if you'd find it useful if I paste a copy of the PHPCS output here.\r\n\r\n#### [PHPCS] Added PHPCompatibility checks to the travis configuration.\r\n* These sniffs check for PHP cross-version compatibility (mainly checks for use of added/removed PHP features).\r\n@see https://github.com/wimg/PHPCompatibility/\r\n* The best way to run this is on the highest available PHP and to test specifically for the lowest version you need the code to run on, which is how I've set it up now.\r\n\r\n#### [Best practices] Applied some memory use improvements\r\n* In straight `echo` statements, there is no need to concatenate. Separating the parts using a comma will archive the same effect and can dramatically reduce the memory use.\r\n@see https://github.com/dseguy/clearPHP/blob/master/rules/no-unnecessary-string-concatenation.md (Great resource to read through completely when you have some time! @dseguy really knows what he's talking about, you can trust me on that.)\r\n* Similarly quite often a `sprintf()` or `printf()` can be used instead of using concatenation.\r\n* Improved some of the layout of the html output while I was at it anyway\r\n\r\n#### [Best practices] Checked for unnecessary use of the `global` keyword\r\n* If the variable was only used once in the context, replace with `$GLOBALS[...]`\r\n* Removed some `global $...` statements where the variable wasn't even used\r\n\r\n__Rationale__: Using the `global` keyword easily creates variable contamination and the use should be minimized, if not avoided.\r\n\r\n#### [Best practices] Combined `isset()` statements\r\n* `isset()` can test several variables in one go, so if in one condition, several (positive) isset's were used, they've been combined into one, as in `isset( $a, $b )` rather than `isset( $a ) && isset( $b )`. After all, one call is faster than two.\r\n\r\n#### [Best practices] Unset variables after use\r\n* Did a cursory walk-through checking that temporary variables - such as key/value in `foreach()` statements, `$i`s in `for()` statements, `$match` from `preg_match()` - are unset after use to prevent use of contaminated variables.\r\n* Also revised/removed a number of existing `unset()` statements which hadn't been updated for the changed codebase.\r\n\r\n#### Also:\r\n* Tried to make the travis script a bit quicker by only cloning the PHPCS libraries when needed [edit: and limiting the history to clone].\r\n* Minor efficiency tweaks\r\n* Fixed some very minor bugs\r\n\t- Tour 'files' link was being pointed to the wrong place.\r\n\t- A conditional in the description theme test was incomplete\r\n\t- Some output escaping\r\n\t- A filter in sitemaps was passing the wrong variable\r\n* Removed an unused method which had been commented out for over six months now.\r\n* Move numerous utility files to the (new) `bin` directory\r\n\t- There are a number of grunt/jshint/csscomb files which are also candidates to be moved, but I wasn't sure whether that would work properly, so I leave that up to @omarreiss to decide upon."},"comment":{"url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/comments/74703620","html_url":"https://github.com/Yoast/wordpress-seo/pull/1919#issuecomment-74703620","issue_url":"https://api.github.com/repos/Yoast/wordpress-seo/issues/1919","id":74703620,"user":{"login":"Rarst","id":737584,"avatar_url":"https://avatars.githubusercontent.com/u/737584?v=3","gravatar_id":"","url":"https://api.github.com/users/Rarst","html_url":"https://github.com/Rarst","followers_url":"https://api.github.com/users/Rarst/followers","following_url":"https://api.github.com/users/Rarst/following{/other_user}","gists_url":"https://api.github.com/users/Rarst/gists{/gist_id}","starred_url":"https://api.github.com/users/Rarst/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Rarst/subscriptions","organizations_url":"https://api.github.com/users/Rarst/orgs","repos_url":"https://api.github.com/users/Rarst/repos","events_url":"https://api.github.com/users/Rarst/events{/privacy}","received_events_url":"https://api.github.com/users/Rarst/received_events","type":"User","site_admin":false},"created_at":"2015-02-17T16:57:14Z","updated_at":"2015-02-17T16:57:14Z","body":"A lot (if not all) code style changes from here (other than those that change code) are incorporated in #1992 with more on top.\r\n\r\nOnce that is merged I can do one final pass on this to extract and deal with anything lingering. :) And after that the state should be good to switch to yoastcs package and new stricter rules."}}