Skip to content

Commit

Permalink
Defined coding standards and applied in select places
Browse files Browse the repository at this point in the history
  • Loading branch information
rimi-itk committed May 1, 2024
1 parent 9f70e50 commit 80552a8
Show file tree
Hide file tree
Showing 14 changed files with 248 additions and 72 deletions.
19 changes: 19 additions & 0 deletions .markdownlintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
// @see https://github.com/DavidAnson/markdownlint/blob/main/schema/.markdownlint.jsonc
// https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md
"MD013": {
// Exclude code blocks
"code_blocks": false,
"line_length": 120
},

// Prevent complaining on duplicated headings in CHANGELOG.md
// https://github.com/DavidAnson/markdownlint/blob/main/doc/md024.md
"MD024": {
"siblings_only": true
}
}

// Local Variables:
// mode: json
// End:
50 changes: 46 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,22 +1,26 @@
# OS2Web Data lookup [![Build Status](https://travis-ci.org/OS2web/os2web_datalookup.svg?branch=8.x)](https://travis-ci.org/OS2web/os2web_datalookup)

## Install

OS2Web Data lookup provides integration with Danish data lookup services such as Service platformen or Datafordeler.
Module is available to download via composer.
```

```shell
composer require os2web/os2web_datalookup
drush en os2web_datalookup
```

## Update

Updating process for OS2Web Data lookup module is similar to usual Drupal 8 module.
Use Composer's built-in command for listing packages that have updates available:

```
```shell
composer outdated os2web/os2web_datalookup
```

## Automated testing and code quality

See [OS2Web testing and CI information](https://github.com/OS2Web/docs#testing-and-ci)

## Contribution
Expand All @@ -28,14 +32,16 @@ For issue description there is expected that you will provide clear and
sufficient information about your feature request or bug report.

### Code review policy

See [OS2Web code review policy](https://github.com/OS2Web/docs#code-review)

### Git name convention

See [OS2Web git name convention](https://github.com/OS2Web/docs#git-guideline)

### Using services in other modules

```
```php
// CVR lookup
/** @var \Drupal\os2web_datalookup\Plugin\DataLookupManager $pluginManager */
$pluginManager = \Drupal::service('plugin.manager.os2web_datalookup');
Expand Down Expand Up @@ -66,7 +72,7 @@ if ($cprPlugin->isReady()) {

## New services/features

### Datafordeler integration (https://datafordeler.dk)
### Datafordeler integration (<https://datafordeler.dk>)

In scope of os2forms project already implemented light integration
with Danmarks Adresseregister (DAR) via fetching data for form elements
Expand All @@ -76,7 +82,9 @@ As soon as it is clear how the integration is going to be used, then
os2forms_dawa will be refactored to OS2Web Data lookup plugin plugin.

## Important notes

### Serviceplatformen plugins

Settings for CPR and CVR serviceplantormen plugins are storing as configuration
in db and will(could) be exported as `yml` file via Drupal configuration
management system. And afterwards could be tracked by `git`.
Expand All @@ -87,3 +95,37 @@ will be exposed for third persons.
To avoid/prevent this behavior we recommend use `Config ignore` module, where
you can add all settings you do not want to export/import via configuration
management system.

## Coding standards

Our coding are checked by GitHub Actions (cf.
[.github/workflows/pr.yml](.github/workflows/pr.yml)). Use the commands below to
run the checks locally.

### PHP

```shell
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer install
# Fix (some) coding standards issues
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-apply
# Check that code adheres to the coding standards
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check
```

### Markdown

```shell
docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' --fix
docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md'
```

## Code analysis

We use [PHPStan](https://phpstan.org/) for static code analysis.

Running statis code analysis on a standalone Drupal module is a bit tricky, so we use a helper script to run the
analysis:

```shell
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm ./scripts/code-analysis
```
40 changes: 36 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
"minimum-stability": "dev",
"prefer-stable": true,
"license": "EUPL-1.2",
"require": {
"ext-soap": "*",
"os2web/os2web_key": "dev-os2web_key"
},
"repositories": {
"os2web/os2web_key": {
"type": "vcs",
Expand All @@ -22,5 +18,41 @@
"type": "composer",
"url": "https://asset-packagist.org"
}
},
"require": {
"ext-soap": "*",
"os2web/os2web_key": "dev-os2web_key"
},
"require-dev": {
"dealerdirect/phpcodesniffer-composer-installer": "*",
"drupal/coder": "*",
"mglaman/phpstan-drupal": "*",
"phpstan/extension-installer": "*",
"phpstan/phpstan-deprecation-rules": "*"
},
"scripts": {
"coding-standards-check/phpcs": [
"phpcs --standard=phpcs.xml.dist"
],
"coding-standards-check": [
"@coding-standards-check/phpcs"
],
"coding-standards-apply/phpcs": [
"phpcbf --standard=phpcs.xml.dist"
],
"coding-standards-apply": [
"@coding-standards-apply/phpcs"
]
},
"config": {
"sort-packages": true,
"allow-plugins": {
"cweagans/composer-patches": true,
"dealerdirect/phpcodesniffer-composer-installer": true,
"phpstan/extension-installer": true,
"simplesamlphp/composer-module-installer": true,
"vaimo/composer-patches": true,
"zaporylie/composer-drupal-optimizations": true
}
}
}
22 changes: 22 additions & 0 deletions phpcs.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="PHP_CodeSniffer">
<description>OS2Forms PHP Code Sniffer configuration</description>

<file>.</file>
<exclude-pattern>vendor/</exclude-pattern>
<exclude-pattern>node_modules/</exclude-pattern>

<!-- Show progress of the run -->
<arg value="p"/>

<arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,yml"/>
<config name="drupal_core_version" value="9"/>

<rule ref="Drupal">
<!-- We want to be able to use "package" and "version" in our custom modules -->
<exclude name="Drupal.InfoFiles.AutoAddedKeys.Project"/>
<exclude name="Drupal.InfoFiles.AutoAddedKeys.Version"/>
</rule>

<rule ref="DrupalPractice"/>
</ruleset>
13 changes: 13 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
parameters:
level: 6
paths:
- src

ignoreErrors:
- "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ has parameter \\$[a-zA-Z0-9_]+ with no value type specified in iterable type array#"
- "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ return type has no value type specified in iterable type array#"
- '#Unsafe usage of new static\(\).#'

# Local Variables:
# mode: yaml
# End:
47 changes: 47 additions & 0 deletions scripts/code-analysis
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env bash
script_dir=$(pwd)
module_name=$(basename "$script_dir")
drupal_dir=vendor/drupal-module-code-analysis
# Relative to $drupal_dir
module_path=web/modules/contrib/$module_name

cd "$script_dir" || exit

drupal_composer() {
composer --working-dir="$drupal_dir" --no-interaction "$@"
}

# # Create new Drupal 9 project
# if [ ! -f "$drupal_dir/composer.json" ]; then
# composer --no-interaction create-project drupal/recommended-project:^9 "$drupal_dir"
# fi
# # Copy our code into the modules folder

# # Clean up
# rm -fr "${drupal_dir:?}/$module_path"

# # https://stackoverflow.com/a/15373763
# # rsync --archive --compress . --filter=':- .gitignore' --exclude "$drupal_dir" --exclude .git "$drupal_dir/$module_path"

# # The rsync command in not available in itkdev/php8.1-fpm

# git config --global --add safe.directory /app
# # Copy module files into module path
# for f in $(git ls-files); do
# mkdir -p "$drupal_dir/$module_path/$(dirname "$f")"
# cp "$f" "$drupal_dir/$module_path/$f"
# done

# drupal_composer config minimum-stability dev

# # Allow ALL plugins
# # https://getcomposer.org/doc/06-config.md#allow-plugins
# drupal_composer config --no-plugins allow-plugins true

# drupal_composer require wikimedia/composer-merge-plugin
# drupal_composer config extra.merge-plugin.include "$module_path/composer.json"
# # https://www.drupal.org/project/drupal/issues/3220043#comment-14845434
# drupal_composer require --dev symfony/phpunit-bridge

# Run PHPStan
(cd "$drupal_dir" && vendor/bin/phpstan --configuration="$module_path/phpstan.neon")
6 changes: 4 additions & 2 deletions src/Exception/RuntimeException.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

namespace Drupal\os2web_datalookup\Exception;

class RuntimeException extends \RuntimeException
{
/**
*
*/
class RuntimeException extends \RuntimeException {

}
29 changes: 16 additions & 13 deletions src/Plugin/os2web/DataLookup/DataLookupBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,25 @@ abstract class DataLookupBase extends PluginBase implements ContainerFactoryPlug
/**
* {@inheritdoc}
*/
public function __construct(array $configuration, $plugin_id, $plugin_definition,
public function __construct(
array $configuration,
$plugin_id,
$plugin_definition,
protected KeyRepositoryInterface $keyRepository,
protected FileSystem $fileSystem
protected FileSystem $fileSystem,
) {
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->setConfiguration($configuration);
$this->init();
}

public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition)
{
/** @var KeyRepositoryInterface $keyRepository */
/**
*
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
/** @var \Drupal\key\KeyRepositoryInterface $keyRepository */
$keyRepository = $container->get('key.repository');
/** @var FileSystem $fileSystem */
/** @var \Drupal\Core\File\FileSystem $fileSystem */
$fileSystem = $container->get('file_system');

return new static(
Expand All @@ -72,11 +77,10 @@ public static function create(ContainerInterface $container, array $configuratio
/**
* Plugin init method.
*/
protected function init()
{
protected function init() {
}

/**
/**
* {@inheritdoc}
*/
public function label() {
Expand Down Expand Up @@ -152,7 +156,7 @@ protected function getCertificate(): string {
* The local certificate path.
*/
protected function createLocalCertPath(): string {
$this->localCertPath = $this->fileSystem->getTempDirectory().'/'.uniqid('os2web_datalookup_local_cert_');
$this->localCertPath = $this->fileSystem->getTempDirectory() . '/' . uniqid('os2web_datalookup_local_cert_');

return $this->localCertPath;
}
Expand All @@ -161,10 +165,9 @@ protected function createLocalCertPath(): string {
* Write certificate to temporary certificate file.
*
* @return string
* The local certificate path.
* The local certificate path.
*/
protected function writeCertificateToFile(): string
{
protected function writeCertificateToFile(): string {
// Write certificate to local_cert location.
$certificate = $this->getCertificate();
$localCertPath = $this->localCertPath;
Expand Down
Loading

0 comments on commit 80552a8

Please sign in to comment.