Coding Standards

[netniV]

As more and more people look to contribute their additions to your code, you may want to define a coding standard. Just looking at the WeatherMapCactiManagementPlugin.php file, I noticed several differing styles of coding for function definitions. I know your wiki has mention of using PSR2 briefly, but often it's worth having a page that spells things out.

[howardjones]

There is this, but it doesn't (yet) mention coding standards. I mostly follow PSR1/PSR2, except for the line length restriction. It would indeed be good to write one up. The phpcs rules I use are in the root directory.

[howardjones]

The other one, especially for core features (not so much for UI, since I haven't figured out a nice way), is to try to add a test for a new bug, as well as the fix (e.g. Alex's NOTES bug the other day)

[netniV]

There is this, but it doesn't (yet) mention coding standards. I mostly follow PSR1/PSR2, except for the line length restriction. It would indeed be good to write one up. The phpcs rules I use are in the root directory.

Not quite sure what "this" is you are referring to :)

Some examples of the different styles:

public function makeURL($params, $altURL = "")
{

    public
    function handleDumpMapsAPI(
        $request,
        $appObject
    ) {

I can submit .a PR to correct these, or would you rather I just left it with you?

[howardjones]

D'oh. I didn't paste the link :-)

THIS https://github.com/howardjones/network-weathermap/blob/master/dev/dev-README.md

[howardjones]

I do use phpcs/phpcbf to check for issues, although I obviously haven't done for a while, because I just had to fix the config to make it work :-) That checks for PSR2 stuff, PHP 7 compatibility stuff and various other things (I have a bad habit of commenting out code rather than trusting the VC, for example). It does take a little bit of management though, as the folks writing these standards tend to only think about the latest PHP release, and I still want to support PHP 5.6 for at least the next release. So for example, I'm just going through disabling (for now) some of the Slevomat/CodingStandard stuff that is PHP 7 specific.

[howardjones]

Having run it again for the first time in a while, the other thing is that it'll never run clean in any of the Cacti-related code, because Cacti doesn't use PSR2, so there will always be a bunch of:

202 | ERROR | [ ] Variable "user_auth_realm_filenames" is not in valid camel caps format

for things defined in Cacti's config.h. This isn't a problem, just an observation, and a reason not to use phpcs as any kind of formal requirement for new code.

[netniV]

True. The Cacti Group code does seem to be based around using underscores for the most part. You would have to be selective as to which rules to apply I guess. We have had to do a similar thing for Markdown with the documentation we have done recently. It flags an error with the markdown lint if you jump more than one level for a subtitle but we use ###### for captions of tables/images so they can be directly linkable.

[howardjones]

I'm pretty sure I can exclude particular rules per file, or just exclude the Integrations/Cacti directory from the normal phpcs pass. I've just run into another fun one, where one of the phpcs standards I used are now marked as requiring PHP 7.1 or above in composer, which is mostly not a problem for a dev environment, but means that only half of the Travis builds work. Having Travis test with various PHP versions is very useful, so goodbye slevomat/coding-standards!

[netniV]

Always fun. You could always fork and make your own?

[howardjones]

Fixed - I made a separate composer.json just for Travis. I've also run phpcbf on everything this morning, to mostly normalise the formatting. Remaining phpcs complaints are almost all about variable naming for Cacti global variables (i.e. something I can't change).