search

ganglia / ganglia-web

Ganglia Web Frontend
BSD 3-Clause "New" or "Revised" License
316 stars 168 forks source link

Fix the Travis build #285

Closed afbjorklund closed 8 years ago

afbjorklund commented 8 years ago

Make PHP Code Sniffer and PHP Unit Testing pass, by editing code standards and fixing code.

afbjorklund commented 8 years ago

Sadly, the pear / pyrus support has been removed from PHP 7.0/nightly in Travis: https://github.com/travis-ci/travis-ci/issues/5256 So we will need to get PHP_CodeSniffer from some other place, for those. Still works OK for PHP 5.5/5.6

afbjorklund commented 8 years ago

@jbuchbinder : could you review the new "coding standard" ? 546afe7b529f9cb07fa6aa9556006535d1bc6cb6

Interestingly, the Generic ruleset actually says the following: A collection of generic sniffs. This standard is not designed to be used to check code.

Maybe some of the other standards should be used instead... http://www.squizlabs.com/php-codesniffer/analysis-of-coding-conventions

afbjorklund commented 8 years ago

@vvuksan : you might want to have a peek at this, especially the test fix in 113f9f31cb64edbe183a9efd0efae6795172cb10 ?

jbuchbinder commented 8 years ago

@itensionanders : It looks pretty good to me. There are a few places where the code fixes are a little ugly, but that's it. Is everyone okay with the Makefile task names?

afbjorklund commented 8 years ago

@jbuchbinder : I'm sure it can be improved manually, I just fixed the warnings/errors - but since the code "standard" doesn't say anything about whitespace, the indentation and trailing whitespace is pretty bad...

i.e. most of the code changes were done automatically by "make fix", in 1a684914189d182b12b0c56e06ecc8ea368fb9ae

As for the make targets, both "sniff" (for Code Sniffer) and "fix" (for Code Beautifier and Fixer) names were kinda made up - just wanted to make them short and memorable. And remove logic from Travis config.

The main goal was just to make all the automated tests (Travis CI) pass again. :-)

afbjorklund commented 8 years ago

Can go with a more boring/crypting "cs" and "cbf", if preferred ? And clean up the whitespace, I suppose.

jbuchbinder commented 8 years ago

It's good for now. We can always adjust whitespace and task names if anyone raises any objections.

afbjorklund commented 8 years ago

Thank you, and yes I'm also sure that some of the stuff that I "excluded" can also be added again - if someone is willing to do the necessary refactorings and cleanup. But now the Travis builds should work again, it was sad that they removed support for both of pear and pyrus just like that - but all good now.