Skip to:
Content

BuddyPress.org

Opened 3 years ago

Last modified 19 months ago

#7228 reopened enhancement

Add PHPCS

Reported by: DJPaul Owned by:
Milestone: Awaiting Contributions Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Cc: contato@…

Description (last modified by netweb)

In order to help ensure a baseline quality of new code contributions and changes, I'm suggesting we think about adopting PHP Code Sniffer (PHPCS).

PHP_CodeSniffer is a PHP script that tokenises and "sniffs" PHP... to detect violations of a defined coding standard. It is an essential development tool that ensures your code remains clean and consistent. It can also help prevent some common semantic errors made by developers.

There are pretty good (though not perfect) WordPress-specific "sniffs" at https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards

In a perfect world, we would have these as a pre-commit hook so Committers don't get lazy, but initially, I assume they'd be hooked up as a Grunt task. I also imagine we'd run with a very minimal set of sniffs so we don't annoy everyone with trivial stuff until we get used to it and/or we all see the value.

Probably the biggest issue is having these code sniffs only report issues with new/changed code, not existing code (our older code is generally not up to the standards of our more recent code), but there are some solutions for this, especially if operating on a Git checkout (see https://github.com/xwp/wp-dev-lib/) though there might be something we can do for SVN checkouts as well.

What do people think?

See also :#WP41057

Change History (11)

#1 @netweb
3 years ago

+1

I believe @tw2113's awesome work on the docs was helped along using PHPCS and https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards, maybe wiring up PHPCS for only docs initially would also help in getting it up and running without annoying everyone, then start enabling some sniffs and plucking what's needed from the also awesome "wp-dev-lib" library.

#2 @tw2113
3 years ago

Most definitely was, awesome tool to help me out.

#3 @DJPaul
3 years ago

@tw2113 want to share the sniffs config you had for that?

#4 @tw2113
3 years ago

I just used the copies that come with VVV, no custom tweaking done to them.

#5 follow-up: @DJPaul
2 years ago

  • Milestone Future Release deleted
  • Resolution set to maybelater
  • Status changed from new to closed

At Human Made, we've had issues with wp-dev-lib and it being quite brittle. Given that wp-dev-lib would be a key part of this - every file would undoubtably through warnings if we weren't running this only on new changes - i'm going to close this ticket for now and if we/Human Made come up with something a bit more reliable and there's interest, we can re-open.

Or switch to Github and have Travis-CI run PHPCS on pull requests. :p

#6 in reply to: ↑ 5 @netweb
2 years ago

Replying to DJPaul:

At Human Made, we've had issues with wp-dev-lib and it being quite brittle. Given that wp-dev-lib would be a key part of this - every file would undoubtably through warnings if we weren't running this only on new changes - i'm going to close this ticket for now and if we/Human Made come up with something a bit more reliable and there's interest, we can re-open.

I agree, it's also about to become a lot less generic, it's currently implementing coding standards changes that differ to WordPress Coding Standards.

Or switch to Github and have Travis-CI run PHPCS on pull requests. :p

Or just run PHPCS across the entire codebase and be done with it, WordPress itself is doing it in #WP41057

WordPress will also be adding pre-commit hooks as part of this, I've some initial work done for this that looks promising

Anyways, I'll keep BP in mind as I will do for bbPress as this WordPress initiative progresses, when I've got some solid plans I'll reopen this ticket with a proposal.

This ticket was mentioned in Slack in #buddypress by netweb. View the logs.


2 years ago

#8 @espellcaste
2 years ago

  • Cc contato@… added

#9 @DJPaul
21 months ago

  • Milestone set to Awaiting Contributions
  • Resolution maybelater deleted
  • Status changed from closed to reopened

#10 @DJPaul
21 months ago

To help audit the Nouveau templates, I've ran a modified version of WP's PHPCS config on it. My current config is at https://gist.github.com/paulgibbs/9d6c1f1cfc5d4765852c0e38a711e1e5

There are some temporary rule exclusions purely to make the rest more manageable, but there's still a bunch of style issues I'm not sure what the correct way to resolve.

e.g. the indentation for the paired template "bp_get_x" functions, and:

-                               $data = wp_parse_args( $style, array(
-                                       'dependencies' => array(),
-                                       'version'      => $this->version,
-                                       'type'         => 'screen',
-                               ) );
+                               $data = wp_parse_args(
+                                       $style, array(
+                                               'dependencies' => array(),
+                                               'version'      => $this->version,
+                                               'type'         => 'screen',
+                                       )
+                               );
-<h2 class="bp-screen-title <?php if ( bp_is_group_create() ) { echo esc_attr( 'creation-step-name' ); } ?>">
+<h2 class="bp-screen-title
+<?php
+if ( bp_is_group_create() ) {
+       echo esc_attr( 'creation-step-name' );
+}
+?>
+">

This is not going to be a trivial thing to adopt to BuddyPress.

#11 @netweb
19 months ago

  • Description modified (diff)
Note: See TracTickets for help on using tickets.