Update Core to comply to coding standards #8

pento · 2017-11-23T06:55:38Z

ntwb · 2017-11-23T09:30:08Z

phpcs.xml.dist

+
+	<!--
+	phpcbf -.-standard=phpcs8.xml.dist -.-report-summary -.-report-source -.-report-full=./20170926-11-phpcbf-all-incl-new-sniffs.txt -.-basepath=I:/000_GitHub/WP_test_suite/
+	-->


The above 3 line comment should be removed, or updated to reference a suggested command line example, e.g. phpcbf --standard=phpcs.xml.dist --report-summary --report-source

ocean90 · 2017-11-23T10:00:17Z

src/wp-admin/about.php

-				'https://github.com/WordPress/gutenberg' ); ?></p>
+				'https://github.com/WordPress/gutenberg'
+			);
+				?>


The indent doesn't seem right here.

ocean90 · 2017-11-23T10:01:11Z

src/wp-admin/admin.php

 		exit;

-	/**
+		/**


WordPress/WordPress-Coding-Standards#1120

ocean90 · 2017-11-23T10:04:03Z

src/wp-admin/admin.php

+					'timeout'     => 120,
+					'httpversion' => '1.1',
+				)
+			);


What rule are we following here? Should this be more like this:

$response = wp_remote_get( admin_url( 'upgrade.php?step=1' ), array( 'timeout' => 120, 'httpversion' => '1.1', ) );

Yeah, that looks wrong. Can you report it to the WPCS repo?

I can, but I wasn't sure if that's a rule we have because it's all over the code base now. Or should I create one for discussion first?

If it doesn't look right, it's probably a bug. ;)

ocean90 · 2017-11-23T10:04:57Z

src/wp-admin/async-upload.php

+				$id, array(
+					'send'   => false,
+					'delete' => true,
+				)


See above, another example:

echo get_media_item( $id, array( 'send' => false, 'delete' => true, ) );

ocean90 · 2017-11-23T10:09:10Z

src/wp-admin/edit-tag-form.php

 if ( 'category' == $taxonomy ) {
 	/**
- 	 * Fires before the Edit Category form.
+	  * Fires before the Edit Category form.


This probably needs manual fixing, there's actually a space before the tab but now it's after the tab. 🤔

WordPress/WordPress-Coding-Standards#977

ocean90 · 2017-11-23T10:11:13Z

src/wp-admin/export.php

 				case 'pages': $('#page-filters').slideDown(); break;
 			}
- 		});
+		 });


Same as above, the extra whitespaces don't get removed, just moved after the tab.

ocean90 · 2017-11-23T10:12:04Z

src/wp-admin/import.php

+								<?php
+								/* translators: %s: importer slug */
+								printf( __( 'The %s importer is invalid or is not installed.' ), '<strong>' . esc_html( $_GET['invalid'] ) . '</strong>' );
+		?>


That's too much indentation.

ocean90 · 2017-11-23T10:18:22Z

tools/i18n/extract.php

@@ -9,7 +9,7 @@
 */
 class StringExtractor {

-	var $rules = array(
+	var $rules          = array(


Not sure if we have to update tools/i18n/ here too. 🤷‍♂️

Did you run the tests manually?

That's the expected behaviour, but it looks a bit weird. Move the $comment_prefix declaration above $rules, or add a blank line between them.

Run what tests manually?

I meant the whole tools/i18n directory.

The tests in https://core.trac.wordpress.org/browser/trunk/tools/i18n/t.

Given that I've never opened that directory before... no. Why doesn't it run in Travis?

I was never a fan of having that directory on core.trac at all. For me the canonical source is still https://i18n.trac.wordpress.org/browser/tools.

Let's just external one of them.

…site-icon Custom properties, site-icon file

…WP_Scripts::localize()`. This function was previously already problematic as it does not do proper input validation, and it has already received tweaks related to PHP 8.0 in [50408] / #52534, which also introduced a `_doing_it_wrong()` notice and added tests. The short of it is: * The function expects to receive an `array` for the `$l10n` parameter; * ...but silently supported the parameter being passed as a `string`; * ...and would expect PHP to gracefully handle everything else or throw appropriate warnings/errors. In the previous fix, a `_doing_it_wrong()` notice was added for any non-array inputs. The function would also cause a PHP native "Cannot use a scalar value as an array" warning (PHP < 8.0) or error (PHP 8.0+) for all scalar values, except `false`. PHP 8.1 deprecated autovivification from `false` to `array`, so now `false` starts throwing an "Automatic conversion of false to array is deprecated" notice. By rights, the function should just throw an exception when a non-array/string input is received, but that would be a backward compatibility break. So the current change will maintain the previous behavior, but will prevent both the "Cannot use a scalar value as an array" warning/error as well as the "Automatic conversion of false to array" deprecation notice for invalid inputs. Invalid inputs ''will'' still receive a `_doing_it_wrong()` notice, which is the reason this fix is considered acceptable. Includes: * Adding a test passing an empty array. * Adding a test to the data provider for a `null` input to make sure that the function will not throw a PHP 8.1 "passing null to non-nullable" notice. This solves the following PHP 8.1 test error: {{{ Tests_Dependencies_Scripts::test_wp_localize_script_data_formats with data set #8 (false, '[""]') Automatic conversion of false to array is deprecated /var/www/src/wp-includes/class.wp-scripts.php:514 /var/www/src/wp-includes/functions.wp-scripts.php:221 /var/www/tests/phpunit/tests/dependencies/scripts.php:1447 /var/www/vendor/bin/phpunit:123 }}} Reference: [https://www.php.net/manual/en/migration81.deprecated.php#migration81.deprecated.core.autovivification-false PHP Manual: PHP 8.1 Deprecations: Autovivification from false]. Follow-up to [7970], [18464], [18490], [19217], [50408]. Props jrf, costdev. See #55656. git-svn-id: https://develop.svn.wordpress.org/trunk@54142 602fd350-edb4-49c9-b593-d223f7449a82

…updates Test updates.

ntwb suggested changes Nov 23, 2017

View reviewed changes

ocean90 reviewed Nov 23, 2017

View reviewed changes

Merge master changes

7d9dc4f

pento force-pushed the 41057-coding-standards branch from 313b7c3 to 7d9dc4f Compare November 30, 2017 19:49

pento closed this Dec 1, 2017

pento deleted the 41057-coding-standards branch December 1, 2017 14:57

circlecube pushed a commit to circlecube/wordpress-develop that referenced this pull request Aug 16, 2021

Merge pull request WordPress#8 from circlecube/try/custom-properties-…

49f2c40

…site-icon Custom properties, site-icon file

toddlahman mentioned this pull request Apr 28, 2023

Cast (int) $cron->time - time() to prevent String - int error #4398

Closed

QAQQL mentioned this pull request Dec 15, 2023

Fix docblock reference in _register_theme_block_patterns #5649

Closed

azaozz pushed a commit to azaozz/wordpress-develop that referenced this pull request Feb 2, 2024

Merge pull request WordPress#8 from costdev/plugin-dependencies-test-…

4335970

…updates Test updates.

               		exit;
-              	/**
+              		/**

Update Core to comply to coding standards #8

Update Core to comply to coding standards #8

Uh oh!

Conversation

pento commented Nov 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants