backdrop-ops / docs.backdropcms.org

Website for displaying Backdrop CMS documentation and API source code.
https://docs.backdropcms.org/
6 stars 6 forks source link

Multiple issue with our coding standard documentation #107

Open indigoxela opened 3 years ago

indigoxela commented 3 years ago

Our coding standard docs need some loving care.

First of all the example for the file docblock misses the empty line after the opening php tag:

<?php
/**
 * @file
 * Foo file does bar.
 */

Many of us use phpcs to verify coding standards, and the Drupal variant of coder requires that empty line, while the backdrop variant tolerates when it's missing.

Worse IMO is the section "Persistent Variables":

Persistent variables (variables/settings defined using Backdrop's variable_get()/variable_set() functions) should be...

In Backdrop variable_set/get is deprecated. The docs shouldn't list these, but provide config_set/get examples.

ghost commented 3 years ago

First of all the example for the file docblock misses the empty line after the opening php tag:

The file docblock should not have a blank line before it. Our coding standards don't "tolerate" the missing blank line, they mandate it:

The / @file / docblock at the beginning of a PHP file should not be preceded by a blank line, but should be followed by a blank line.

(https://api.backdropcms.org/php-standards#indenting (emphasis mine))

If the Coder module doesn't flag a blank line before the @file docblock, then the Coder module should be updated to match our coding standards, not the other way around.

indigoxela commented 3 years ago

Another mismatch of coder sniffer for Backdrop and our docs...

phpcs nags: "Comments may not appear after statements." but they may.

What's the use of a tool, that validates different stuff than what's documented?

ghost commented 3 years ago

Another mismatch of coder sniffer for Backdrop and our docs... ... What's the use of a tool, that validates different stuff than what's documented?

Yep. I think there are two reasons for this:

  1. Nothing is perfect. There will always be bugs, oversights, etc.
  2. The PHPCS code for Backdrop core lives in a contrib module. It's therefore up to the contrib module maintainer to make changes, keep it up-to-date, etc. For this reason, I have suggested that the PHPCS stuff be separated out into a separate repo in backdrop-ops. Not only will it then be easier to keep up-to-date, but modules that require it (like Coder Review or Drush) can depend on it without needing to duplicate it. Basically we should do the same thing @serundeputy has done but in a more official capacity (or just ask if he can move his repo under backdrop-ops).