Make WordPress Core

Opened 5 years ago

Closed 18 months ago

Last modified 18 months ago

#50010 closed task (blessed) (fixed)

wp-includes/blocks shouldn't be excluded from coding standards

Reported by: johnbillion's profile johnbillion Owned by: johnbillion's profile johnbillion
Milestone: 6.4 Priority: normal
Severity: normal Version: 5.0
Component: Editor Keywords: good-first-bug needs-testing has-patch commit early
Focuses: coding-standards Cc:

Description

The wp-incudes/blocks directory is excluded from coding standards. This is presumably because the files are imported from Gutenberg, but there's no reason these files shouldn't adhere to core's coding standards.

Needs some investigation into why the directory is excluded, whether the files need updating in Gutenberg to adhere to core coding standards, and corresponding tickets on the Gutenberg repo as necessary.

Change History (9)

#1 @noisysocks
4 years ago

  • Milestone changed from Awaiting Review to Future Release
  • Type changed from enhancement to task (blessed)

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


4 years ago

#3 @lopo
3 years ago

I tried to enable CS checks on the folder, and we get just four sets of errors like:

 1 | ERROR | [x] When a multi-item array uses associative keys, each value should start on a new line.
   |       |     (WordPress.Arrays.ArrayDeclarationSpacing.AssociativeArrayFound)
 1 | ERROR | [x] File must end with a newline character (Generic.Files.EndFileNewline.NotFound)

on navigation/view.asset.php, navigation/view.min.asset.php, file/view.asset.php and file/view.min.asset.php because they all like this:

<?php return array('dependencies' => array(), 'version' => '3776ea67846b3bb10fe8f7cdd486b0ba');

Obviously they're trivial problems but as far as I know those files are autogenerated, so with my limited knowledge of Gutenberg I'm not sure what can be done about them.

This ticket was mentioned in PR #2644 on WordPress/wordpress-develop by Rahe.


3 years ago
#4

  • Keywords has-patch added

Exclude any asset file from the blocs directory since they are auto-generated.

Trac ticket: https://core.trac.wordpress.org/ticket/50010

#5 @Rahe
3 years ago

Hello,

I've added a PR for this since, as said, the PHP files are auto-generated.
But maybe there will never be any PHP files into theses directories ?

#6 @adamsilverstein
20 months ago

  • Keywords reporter-feedback added

@johnbillion what do you think of the proposed solution that changes the exclusion to only the auto-generated files, not the entire folder? Otherwise the fix probably belongs in Gutenberg, right?

#7 @johnbillion
20 months ago

  • Keywords commit early added; reporter-feedback removed
  • Milestone changed from Future Release to 6.4
  • Owner set to johnbillion
  • Status changed from new to accepted

Sounds good to me.

#8 @johnbillion
18 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56392:

Editor: Only exclude auto-generated block files from coding standards checks.

Prior to this change, all block files were excluded from coding standards checks, but there's no reason these files shouldn't adhere to core's coding standards.

Props lopo, Rahe, adamsilverstein 

Fixes #50010

Note: See TracTickets for help on using tickets.