Test codeclimate

[MPParsley]

You can ignore this PR. It's a test to demonstrate CodeClimate.

[MPParsley]

I enabled code climate and configured some defaults.

Some of these checks may be somewhat rigorous (e.g. the limit for the amount of lines in a method is now 50). This is a good default but obviously this doesn't work for Drupal Form classes. Note that it is possible to dismiss suggestions.

If we plan to enable branch protection with code climate, we need to make sure all contributors have access to dismiss these suggestions.

@amitaibu, can you check if you can dismiss suggestions?

From within the PR:

On codeclimate itself:

[pfrenssen]

I'm starting to see the value in this 😄

Can't you disable the checks on cyclomatic complexity, and the ones on method length and file length? Just increasing the limit doesn't make sense I think.

[MPParsley]

Yes, we should definitely finetune this.

• disabled some of the "dumb" checks (line count etc);
• added some linters from Drupal core;
• we can configure test coverage to measure and enforce it

I hope the infrastructure team will provide something similar on Gitlab but in the meantime we can get some added value from this tool.

Note: I'm not 100% sure but I think that we must merge the .codeclimate.yml file into develop before it takes place.

[pfrenssen]

Wow now it is full of actionable suggestions!! This is very nice!

We could make a whole bunch of novice issues out of this for using at the Drupal Dev Days to do sprint mentoring!

[MPParsley]

Phpunit requires Xdebug and Xdebug makes phpunit a lot slower. So now the tests failed after 50 minutes.

Testing /home/travis/build/Gizra/drupal/modules/og/tests
...................
The job exceeded the maximum time limit for jobs, and has been terminated.

We'll have to do without code coverage for now. 😞

[pfrenssen]

Can we disable PHP_CodeSniffer? This is not needed since this is already being checked, and it is throwing errors: https://codeclimate.com/github/Gizra/og/pull/509

[pfrenssen]

I wouldn't run the tests through CodeClimate either. These are already running very reliably.

Rule #1: if it is not broken, don't fix it.

[pfrenssen]

It's still reporting a lot of non-issues, and it seems the test code is being ignored?

I would disable PHP Mess Detector I have never known this to give useful feedback.

[pfrenssen]

Also, would it be possible to experiment with this on a fork instead of on the main repository? Currently all the ongoing PRs are failing because of this. If we can get it to a workable state on a fork it should be easy to move it over to the main repo after. And then at least we can continue working without interruption.

[MPParsley]

Some overdue feedback;

Can we disable PHP_CodeSniffer? This is not needed since this is already being checked, and it is throwing errors: https://codeclimate.com/github/Gizra/og/pull/509

Preferably not. Having feedback inline in the code on GitHub is an added value over the Travis report.

I wouldn't run the tests through CodeClimate either. These are already running very reliably.

Note that we're never running tests through CodeClimate. We merely send an xml report to CodeClimate.

Rule #1: if it is not broken, don't fix it.

I'll comment on this quote with a few other quotes ;-)

This feature would enable us to:

• enforce partial (percentage) or even full code coverage by tests
• enforce same percentage or more code coverage between PR's
• visible reports and statistics on the progress we're making

It's still reporting a lot of non-issues, and it seems the test code is being ignored?

In order for phpunit to measure test coverage, Xdebug is needed. Unfortunately I ran into this issue in .travis.yml and it takes more than 50 minutes to run all tests on Travis (might be in part due to the amount of Kernel tests but this would need further investigation):

  # Remove Xdebug as we don't need it and it causes "PHP Fatal error: Maximum
  # function nesting level of '256' reached."
  # We also don't care if that file exists or not on PHP 7.

For now I disabled this feature as you requested.

I would disable PHP Mess Detector I have never known this to give useful feedback.

I disabled this feature as you requested but I'm convinced that all this feedback is welcome:

PHPMD looks for several potential problems within PHP source code, like possible bugs, suboptimal code, overcomplicated expressions, and unused parameters, methods, or properties.

If there's still feedback that we consider useless, CodeClimate allows you to simply ignore this.

Also, would it be possible to experiment with this on a fork instead of on the main repository? Currently all the ongoing PRs are failing because of this. If we can get it to a workable state on a fork it should be easy to move it over to the main repo after. And then at least we can continue working without interruption.

I had set it up on my fork to but the errors you see in the other PR's are based on the default configuration of CodeClimate. Until we add the .codeclimate.yml configuration in #505 the default configuration will be used (which has a wrong CS ruleset that report 4 spaces instead of 2 for example).

If you like I can disable the integration completely or we could commit the .codeclimate.yml configuration from #505 which now only reports 63 issues (btw, which is awesome < %5 technical debt). I marked them all as approved so they won't show up in new PR's. We could then resolve them in follow up issues or choose to ignore them and benefit from the feedback from CodeClimate in new PR's.

wdyt?

cc @pfrenssen, @amitaibu, @AronNovak