openeuropa / code-review

Automated quality assurance checks based on GrumPHP.

The OpenEuropa Initiative

OpenEuropa is a Directorate-General for Informatics (DIGIT) initiative aiming at strengthening the adoption of open source tools and practices in consolidating the European Institutions' web presence.

In order to achieve such a goal the OpenEuropa Initiative will focus on the following activities:

• Build, maintain and release loosely-coupled, reusable software components , licensed under EUPL-1.2.
• Build, maintain and release fully-fledged solutions for the European Institutions.
• Provide a high-level architecture overview of web-related information systems.
• Establish reusable technical governance guidelines.
• Contribute back to upstream open source projects.

For more information please check:

• OpenEuropa initiative
  • Code of Conduct
  • Release cycle
  • How to contribute
  • Contribution workflow
  • Components
• Before you start
  • Solution definition
  • Repositories scaffolding
• Development guidelines
  • Content architecture
  • Configuration management
  • Writing tests
  • Module update policy
  • Tooling
  • Dependency management
  • Development Workflow
  • Coding standards
  • Frontend development
  • CI And deployment workflow
  • Code reviews
  • Release workflow
• Templates
  • Issue template
  • Pull request template
• Development guides
  • Drupal
• Automated testing

code-review's Issues

Why is drupal/coder locked to 8.3.9?

Hello,

While upgrading a project, I noticed that release 1.5.0 of openeuropa/code-review locks drupal/coder to 8.3.9:
https://github.com/openeuropa/code-review/blob/1.5.0/composer.json#L8
I was on openeuropa/code-review:1.5.0 and for some reason composer update causes openeuropa/code-review to be downgraded back to 1.4.0.

Is there any good reason for this?
Couldn't we relax a bit the requirement to e.g: "drupal/coder": "^8.3" ?

Thank you,
Hervé

Uncaught TypeError: Argument 2 passed to Symfony\Component\EventDispatcher\EventDispatcher::dispatch() must be an instance of Symfony\Component\EventDispatcher\Event or null

Hi!

After updating the Drupal module Search API Solr to the latest version (4.1.11) which also updates some symfony packages i have an issue with GrumPHP.

When i run the comand ./vendor/bin/grumphp run i got this error:

Fatal error: Uncaught TypeError: Argument 2 passed to Symfony\Component\EventDispatcher\EventDispatcher::dispatch() must be an instance of Symfony\Component\EventDispatcher\Event or null, instance of GrumPHP\Event\RunnerEvent given, called in /var/www/html/vendor/phpro/grumphp/src/Event/Dispatcher/Bridge/SymfonyEventDispatcher.php on line 32 and defined in /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php:37
Stack trace:
#0 /var/www/html/vendor/phpro/grumphp/src/Event/Dispatcher/Bridge/SymfonyEventDispatcher.php(32): Symfony\Component\EventDispatcher\EventDispatcher->dispatch('grumphp.runner....', Object(GrumPHP\Event\RunnerEvent))
#1 /var/www/html/vendor/phpro/grumphp/src/Runner/TaskRunner.php(76): GrumPHP\Event\Dispatcher\Bridge\SymfonyEventDispatcher->dispatch(Object(GrumPHP\Event\RunnerEvent), 'grumphp.runner....')
#2 /var/www/html/vendor/phpro/grumphp/src/Console/Helper/TaskRunnerHelper.php(69): GrumPHP\Runner\TaskRunner->run(Object(GrumPHP\Runner\TaskRunnerContext))
#3 /var/www/html/vendor/phpro/grumphp/src/Console/Command/RunCommand.php(80): GrumPHP\Console\Helper\TaskRunnerHelper->run(Object(Symfony\Component\Console\Output\ConsoleOutput), Object(GrumPHP\Runner\TaskRunnerContext))
#4 /var/www/html/vendor/symfony/console/Command/Command.php(255): GrumPHP\Console\Command\RunCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#5 /var/www/html/vendor/symfony/console/Application.php(1005): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#6 /var/www/html/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(GrumPHP\Console\Command\RunCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#7 /var/www/html/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#8 /var/www/html/vendor/phpro/grumphp/bin/grumphp(50): Symfony\Component\Console\Application->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#9 /var/www/html/vendor/phpro/grumphp/bin/grumphp(51): {closure}()
#10 {main}
  thrown in /var/www/html/vendor/symfony/event-dispatcher/EventDispatcher.php on line 37

Anyone seen this before?

Initial setup

Add grumphp parameters to drupal conventions.

Currently most projects need grumphp configuration like this:

imports:
  - { resource: vendor/openeuropa/code-review/dist/drupal-conventions.yml }
parameters:
  tasks.phpcs.ignore_patterns:
    - build/
    - node_modules/
    - vendor/
  tasks.phpcs.triggered_by:
    - inc
    - install
    - module
    - php
    - theme
    - yml

Since these parameters are common in most projects, they can be added to code-review's drupal conventions.

Update phpro/grumphp

We are currently facing issue phpro/grumphp#487
I have noticed that you have a quite old version of phpro/grumphp do you consider update it at some point ?

Do not use a fixed version for GrumPHP

Add GrumPHP task: "yamllint" to check yaml files syntax.

GrumPHP has a task "yamllint".

The YamlLint task will lint all the yaml files of the project.

Issue when the tasks key is not set.

There is an issue when we use this grumphp.yml.dist file:

imports:
  - { resource: vendor/openeuropa/code-review/dist/drupal-conventions.yml }
parameters:
  extensions:
    - OpenEuropa\CodeReview\ExtraTasksExtension
  tasks: ~
  extra_tasks:
    phpunit:
      always_execute: false
    securitychecker:
      lockfile: ./composer.lock
      format: ~
      end_point: ~
      timeout: ~
      run_always: false

the log:

 ~/d/g/atomium   187-use-docker-for-tests *$…  docker-compose exec -u www-data web ./vendor/bin/grumphp run                        1102ms  Fri 23 Nov 2018 02:03:59 PM CET
PHP Warning:  array_key_exists() expects parameter 2 to be array, null given in /var/www/html/vendor/openeuropa/code-review/src/ExtraTasksExtension.php on line 22
PHP Stack trace:
PHP   1. {main}() /var/www/html/vendor/phpro/grumphp/bin/grumphp:0
PHP   2. GrumPHP\Console\Application->__construct() /var/www/html/vendor/phpro/grumphp/bin/grumphp:30
PHP   3. GrumPHP\Console\Application->getContainer() /var/www/html/vendor/phpro/grumphp/src/Console/Application.php:49
PHP   4. GrumPHP\Configuration\ContainerFactory::buildFromConfiguration() /var/www/html/vendor/phpro/grumphp/src/Console/Application.php:151
PHP   5. Symfony\Component\DependencyInjection\ContainerBuilder->compile() /var/www/html/vendor/phpro/grumphp/src/Configuration/ContainerFactory.php:49
PHP   6. Symfony\Component\DependencyInjection\Compiler\Compiler->compile() /var/www/html/vendor/symfony/dependency-injection/ContainerBuilder.php:788
PHP   7. GrumPHP\Configuration\Compiler\ExtensionCompilerPass->process() /var/www/html/vendor/symfony/dependency-injection/Compiler/Compiler.php:141
PHP   8. OpenEuropa\CodeReview\ExtraTasksExtension->load() /var/www/html/vendor/phpro/grumphp/src/Configuration/Compiler/ExtensionCompilerPass.php:32
PHP   9. array_key_exists() /var/www/html/vendor/openeuropa/code-review/src/ExtraTasksExtension.php:22

Warning: array_key_exists() expects parameter 2 to be array, null given in /var/www/html/vendor/openeuropa/code-review/src/ExtraTasksExtension.php on line 22

Call Stack:
    0.0001     352304   1. {main}() /var/www/html/vendor/phpro/grumphp/bin/grumphp:0
    0.0056    1280120   2. GrumPHP\Console\Application->__construct() /var/www/html/vendor/phpro/grumphp/bin/grumphp:30
    0.0064    1422680   3. GrumPHP\Console\Application->getContainer() /var/www/html/vendor/phpro/grumphp/src/Console/Application.php:49
    0.0393    3828024   4. GrumPHP\Configuration\ContainerFactory::buildFromConfiguration() /var/www/html/vendor/phpro/grumphp/src/Console/Application.php:151
    0.1008    6116888   5. Symfony\Component\DependencyInjection\ContainerBuilder->compile() /var/www/html/vendor/phpro/grumphp/src/Configuration/ContainerFactory.php:49
    0.1019    6123048   6. Symfony\Component\DependencyInjection\Compiler\Compiler->compile() /var/www/html/vendor/symfony/dependency-injection/ContainerBuilder.php:788
    0.1031    6161984   7. GrumPHP\Configuration\Compiler\ExtensionCompilerPass->process() /var/www/html/vendor/symfony/dependency-injection/Compiler/Compiler.php:141
    0.1032    6166984   8. OpenEuropa\CodeReview\ExtraTasksExtension->load() /var/www/html/vendor/phpro/grumphp/src/Configuration/Compiler/ExtensionCompilerPass.php:32
    0.1032    6166984   9. array_key_exists() /var/www/html/vendor/openeuropa/code-review/src/ExtraTasksExtension.php:22

GrumPHP is sniffing your code!
Running task 1/2: Phpunit... ✔
Running task 2/2: SecurityChecker... ✔
 ~/d/g/atomium   187-use-docker-for-tests *$… 

Update GrumPHP to version 0.13

Use PHP CS Fixer

Configuration helper: https://mlocati.github.io/php-cs-fixer-configurator/

Get a PR diff without the whitespace changes: https://github.com/openeuropa/code-review/pull/50/files?utf8=%E2%9C%93&diff=split&w=1

Drop character limit on first line of commit messages

Currently there is an arbitrary limit of 60 characters placed on the first line of the commit message. Combined with our other rules of prefixing the message with OPENEUROPA-1234: and suffixing with a period this just leaves us with 42 usable characters. This is not enough to provide a clear description on why a certain change was necessary.

I would suggest to abolish the character limit entirely. Conveying clear information in the git history is more important than having nice looking titles in Github.

It also seems this rule has never been intentional but has been inherited from GrumPHP defaults. Our current list of conventions for commit messages does not mention an arbitrary character limit.

OPENEUROPA-1575: Add a PHP Lint that checks the PHP syntax before running any other tools.

GrumPHP has a built-in task 'phplint' (php -l) that should be run before other tasks.

The current behavior when PHP code has syntax errors is to just fail without notice.

Thanks to this tool, it will exactly tell people where the issue is.

NEPT-1100_documentation_and_changelog

Documentation should live alongside the code and committed to the same repository
It should be written using Markdown or equivalent metalanguage
(Optional) Documentation should be accessible as a browsable site

TODO
Changelog should be automatically generated starting from Github PR and / or issues

Update squizlabs/php_codesniffer to 3.4.1

We could update squizlabs/php_codesniffer to 3.4.1 and remove the following patch
"Fix conflict between scope indentation and array indentation checking. Ref. https://github.com/squizlabs/PHP_CodeSniffer/issues/2281": "https://gist.githubusercontent.com/drupol/c307b48fe4eb187b0833afb1d1b97e6a/raw/c3d414e9f4c182387ca9405c36f5daeb7b51ae05/phpcs-390bffe2265aac8184fdf86400c0d0841e65bc4b.patch"

Update or delete a task configuration

Add a grumPHP task: "jsonlint" to check json files syntax.

GrumPHP has a task "jsonlint".

The JSONLint task will lint all the json files of the project.

Add a Drupal 7 conventions

The master branch now integrates php-cs-fixer.
The file dist/drupal-conventions.yml is tailored for Drupal 8 development.
We should provide a dist/drupal7-conventions.yml tailored for Drupal 7 so projects like Atomium and EC Europa themes could use it as well.

Add PHPUnit testing coverage

Find a way to normalize the composer.json file.

Using the package from @localheinz : https://github.com/localheinz/composer-normalize

This package doesn't work with PHP < 7.1

See blog post: https://localheinz.com/blog/2018/01/15/normalizing-composer.json/

Add "composer" task to grumphp.

When the composer.json file has changed, the new file should be checked for issues.
This task will execute composer validate to make sure that everything is OK.

Update friendsofphp/php-cs-fixer to v2.15.0

When using friendsofphp/php-cs-fixer in version v2.14.2 we may incour in error "php_unit_test_case_static_method_calls".
See https://drone.fpfis.eu/openeuropa/epoetry-client/254/7
Updating to v2.15.0 fixes the issue.

Add convention files for both Drupal and non-Drupal projects

Scope

The scope of this issue is to add two convention files:

• ./dist/drupal-conventions.yml for anything Drupal-related
• ./dist/library-conventions.yml for libraries and other non-Drupal code

Test if all use statements are used.

A very common error is to forget "use" statements that are not needed anymore.
Currently this is not tested.

OPENEUROPA-1535: Revert.

See #51

Make sure tabs are converted in spaces by adding the rule 'indentation_type' in php-cs-fixer config.

Release 1.0.0-beta3

Add roave/security-advisories to dev dependencies.

Adding this package will ensure that the application using openeuropa/code-review package doesn't have installed dependencies with known security vulnerabilities.

This package can only be required in its dev-master version: there will never be stable/tagged versions because of the nature of the problem being targeted. Security issues are in fact a moving target, and locking your project to a specific tagged version of the package would not make any sense.

This package is therefore only suited for installation in the root of your deployable project.

Add YML file extension in the codestyle check.

Add theme files to oe-code-review default conventions

Currently the Code Review component only runs phpcs on the following file types:

• php
• module
• inc

Files with "theme" extension should also be considered to account for custom themes.

Reconsider having commit messages starting with hash (#)

Having commit messages starting with a hash can be very annoying and is not used in most of the projects. Drupal.org, in Commit messages - providing history and credit, explicitly states against using hash (#) as the first character in commit message:

Important: Do NOT begin your commit message with the # symbol. While this was a best practice with CVS, it will cause Git to remove the commit message entirely in certain situations (e.g. git rebase -i).

The reason is that, sometimes, the commit messages get edited in an editor (for example on interactive rebase). Is such cases all lines prefixed with hash (#) are considered comments and are removed from the commit message.

See also:

• https://stackoverflow.com/questions/2788092/start-a-git-commit-message-with-a-hashmark

Rename project before release

Add PHPMD task and conventions

Expand the existing tasks with PHPMD.

Define which conventions are going to be overridable.