[buddypress-trac] [BuddyPress Trac] #7228: Add PHPCS

[buddypress-trac]

#7228: Add PHPCS
------------------------------+-----------------------
 Reporter:  DJPaul            |       Owner:  (none)
     Type:  enhancement       |      Status:  reopened
 Priority:  normal            |   Milestone:  8.0.0
Component:  Build/Test Tools  |     Version:
 Severity:  normal            |  Resolution:
 Keywords:                    |
------------------------------+-----------------------

Comment (by espellcaste):

 Thanks John! Your feedback is not discouraging at all, on the contrary. I
 have a different take and goal about PHPCS for BuddyPress.

 I see PHPCS not as a ''valueless'' standard, but as an opportunity to
 catch bugs, or at the very least, to be aware of them and create a plan to
 fix them. In the long run, we will be making them visible and hopefully
 fixing them. This is also a good opportunity to set, or follow, some good
 patterns and standards across the projects.

 Some of your concerns I'm not sure we can solve. IDE setup, for example,
 is very tricky and personal. BuddyPress can't, nor should, be responsible
 or be concerned with that. Like you said, this should be for each person
 for figure it out.

 > ... the codebase ends up littered with ugly exception comments anyways –
 the antithesis of cleaner code.

 Another good point. I believe that's inevitable with a project with such a
 history as BuddyPress. But I'd caution here that the goal is not
 perfection or following the ''valueless'' standard by the book. But to
 thrive for a better code base, and at the very least, catch ERRORS as soon
 as possible.

 Will we silent or remove some rules? Yes. But ideally, they would be
 warnings, not errors. But the most important point I could make here is:
 we need to know where the problem lies. Today, only checking against
 `PHPCompatibilityWP`, we can't.

 > It needs to be up to committers to run PHPCS when committing, and
 patches shouldn't be rejected only because of PHPCS issues.

 I disagree here partly. I think that patches, or commits in Github
 projects, should not go through if they have errors in their code. They
 can have warnings, but not errors.

 We can run phpcs: checking for errors, in changed files only. And run it
 completely in the CI. We can also see warnings in the CI and in the
 terminal but not fail the build/commit/release/etc.

 Again, the goal here is to be proactive and catch errors early. They might
 be annoying but they are necessary for a better code base. This is not
 only about following rules or standards.

 ---

 Overall, I don't see this as disruptive as you seem to be. Ideally, people
 will continue to contribute code as they always have been. They might only
 be bothered if they have an error in their code, something I presume to be
 a good thing to catch beforehand.

 My proposal is the creation of a custom BuddyPress ruleset with packages
 and rules that should be shared between the BuddyPress projects (including
 BP itself).

 The projects themselves can exclude or add custom rules as needed or
 desired. But currently everyone is doing or installing their own thing.

 The BP REST API uses a strict WordPress ruleset catching all errors, but
 BuddyPress doesn't. The BP WP-CLI package does their thing, etc. I mean,
 everyone is doing something differently. Having a common standard should
 improve overall code quality by following sensible rules.

 Sharing future changes across the projects will be easier as well with
 every project using this custom package/ruleset.

 Overall, I think BuddyPress would benefit from it. In the same way we
 encourage unit tests as much as possible, we should be encouraging
 following PHPCS as well.

 :)

-- 
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7228#comment:16>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac