Skip to content

Commit c961fbc

Browse files
committed
Defined coding standards and applied in select places
1 parent 7175de0 commit c961fbc

14 files changed

+248
-72
lines changed

.markdownlintrc

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
// @see https://github.com/DavidAnson/markdownlint/blob/main/schema/.markdownlint.jsonc
3+
// https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md
4+
"MD013": {
5+
// Exclude code blocks
6+
"code_blocks": false,
7+
"line_length": 120
8+
},
9+
10+
// Prevent complaining on duplicated headings in CHANGELOG.md
11+
// https://github.com/DavidAnson/markdownlint/blob/main/doc/md024.md
12+
"MD024": {
13+
"siblings_only": true
14+
}
15+
}
16+
17+
// Local Variables:
18+
// mode: json
19+
// End:

README.md

+46-4
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,26 @@
11
# OS2Web Data lookup [![Build Status](https://travis-ci.org/OS2web/os2web_datalookup.svg?branch=8.x)](https://travis-ci.org/OS2web/os2web_datalookup)
2+
23
## Install
34

45
OS2Web Data lookup provides integration with Danish data lookup services such as Service platformen or Datafordeler.
56
Module is available to download via composer.
6-
```
7+
8+
```shell
79
composer require os2web/os2web_datalookup
810
drush en os2web_datalookup
911
```
1012

1113
## Update
14+
1215
Updating process for OS2Web Data lookup module is similar to usual Drupal 8 module.
1316
Use Composer's built-in command for listing packages that have updates available:
1417

15-
```
18+
```shell
1619
composer outdated os2web/os2web_datalookup
1720
```
1821

1922
## Automated testing and code quality
23+
2024
See [OS2Web testing and CI information](https://github.com/OS2Web/docs#testing-and-ci)
2125

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

3034
### Code review policy
35+
3136
See [OS2Web code review policy](https://github.com/OS2Web/docs#code-review)
3237

3338
### Git name convention
39+
3440
See [OS2Web git name convention](https://github.com/OS2Web/docs#git-guideline)
3541

3642
### Using services in other modules
3743

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

6773
## New services/features
6874

69-
### Datafordeler integration (https://datafordeler.dk)
75+
### Datafordeler integration (<https://datafordeler.dk>)
7076

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

7884
## Important notes
85+
7986
### Serviceplatformen plugins
87+
8088
Settings for CPR and CVR serviceplantormen plugins are storing as configuration
8189
in db and will(could) be exported as `yml` file via Drupal configuration
8290
management system. And afterwards could be tracked by `git`.
@@ -87,3 +95,37 @@ will be exposed for third persons.
8795
To avoid/prevent this behavior we recommend use `Config ignore` module, where
8896
you can add all settings you do not want to export/import via configuration
8997
management system.
98+
99+
## Coding standards
100+
101+
Our coding are checked by GitHub Actions (cf.
102+
[.github/workflows/pr.yml](.github/workflows/pr.yml)). Use the commands below to
103+
run the checks locally.
104+
105+
### PHP
106+
107+
```shell
108+
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer install
109+
# Fix (some) coding standards issues
110+
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-apply
111+
# Check that code adheres to the coding standards
112+
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm composer coding-standards-check
113+
```
114+
115+
### Markdown
116+
117+
```shell
118+
docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md' --fix
119+
docker run --rm --volume $PWD:/md peterdavehello/markdownlint markdownlint --ignore vendor --ignore LICENSE.md '**/*.md'
120+
```
121+
122+
## Code analysis
123+
124+
We use [PHPStan](https://phpstan.org/) for static code analysis.
125+
126+
Running statis code analysis on a standalone Drupal module is a bit tricky, so we use a helper script to run the
127+
analysis:
128+
129+
```shell
130+
docker run --rm --volume ${PWD}:/app --workdir /app itkdev/php8.1-fpm ./scripts/code-analysis
131+
```

composer.json

+36-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@
55
"minimum-stability": "dev",
66
"prefer-stable": true,
77
"license": "EUPL-1.2",
8-
"require": {
9-
"ext-soap": "*",
10-
"os2web/os2web_key": "dev-os2web_key"
11-
},
128
"repositories": {
139
"os2web/os2web_key": {
1410
"type": "vcs",
@@ -22,5 +18,41 @@
2218
"type": "composer",
2319
"url": "https://asset-packagist.org"
2420
}
21+
},
22+
"require": {
23+
"ext-soap": "*",
24+
"os2web/os2web_key": "dev-os2web_key"
25+
},
26+
"require-dev": {
27+
"dealerdirect/phpcodesniffer-composer-installer": "*",
28+
"drupal/coder": "*",
29+
"mglaman/phpstan-drupal": "*",
30+
"phpstan/extension-installer": "*",
31+
"phpstan/phpstan-deprecation-rules": "*"
32+
},
33+
"scripts": {
34+
"coding-standards-check/phpcs": [
35+
"phpcs --standard=phpcs.xml.dist"
36+
],
37+
"coding-standards-check": [
38+
"@coding-standards-check/phpcs"
39+
],
40+
"coding-standards-apply/phpcs": [
41+
"phpcbf --standard=phpcs.xml.dist"
42+
],
43+
"coding-standards-apply": [
44+
"@coding-standards-apply/phpcs"
45+
]
46+
},
47+
"config": {
48+
"sort-packages": true,
49+
"allow-plugins": {
50+
"cweagans/composer-patches": true,
51+
"dealerdirect/phpcodesniffer-composer-installer": true,
52+
"phpstan/extension-installer": true,
53+
"simplesamlphp/composer-module-installer": true,
54+
"vaimo/composer-patches": true,
55+
"zaporylie/composer-drupal-optimizations": true
56+
}
2557
}
2658
}

phpcs.xml.dist

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<ruleset name="PHP_CodeSniffer">
3+
<description>OS2Forms PHP Code Sniffer configuration</description>
4+
5+
<file>.</file>
6+
<exclude-pattern>vendor/</exclude-pattern>
7+
<exclude-pattern>node_modules/</exclude-pattern>
8+
9+
<!-- Show progress of the run -->
10+
<arg value="p"/>
11+
12+
<arg name="extensions" value="php,module,inc,install,test,profile,theme,css,info,txt,yml"/>
13+
<config name="drupal_core_version" value="9"/>
14+
15+
<rule ref="Drupal">
16+
<!-- We want to be able to use "package" and "version" in our custom modules -->
17+
<exclude name="Drupal.InfoFiles.AutoAddedKeys.Project"/>
18+
<exclude name="Drupal.InfoFiles.AutoAddedKeys.Version"/>
19+
</rule>
20+
21+
<rule ref="DrupalPractice"/>
22+
</ruleset>

phpstan.neon

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
parameters:
2+
level: 6
3+
paths:
4+
- src
5+
6+
ignoreErrors:
7+
- "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ has parameter \\$[a-zA-Z0-9_]+ with no value type specified in iterable type array#"
8+
- "#Method [a-zA-Z0-9\\_\\\\:\\(\\)]+ return type has no value type specified in iterable type array#"
9+
- '#Unsafe usage of new static\(\).#'
10+
11+
# Local Variables:
12+
# mode: yaml
13+
# End:

scripts/code-analysis

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/usr/bin/env bash
2+
script_dir=$(pwd)
3+
module_name=$(basename "$script_dir")
4+
drupal_dir=vendor/drupal-module-code-analysis
5+
# Relative to $drupal_dir
6+
module_path=web/modules/contrib/$module_name
7+
8+
cd "$script_dir" || exit
9+
10+
drupal_composer() {
11+
composer --working-dir="$drupal_dir" --no-interaction "$@"
12+
}
13+
14+
# # Create new Drupal 9 project
15+
# if [ ! -f "$drupal_dir/composer.json" ]; then
16+
# composer --no-interaction create-project drupal/recommended-project:^9 "$drupal_dir"
17+
# fi
18+
# # Copy our code into the modules folder
19+
20+
# # Clean up
21+
# rm -fr "${drupal_dir:?}/$module_path"
22+
23+
# # https://stackoverflow.com/a/15373763
24+
# # rsync --archive --compress . --filter=':- .gitignore' --exclude "$drupal_dir" --exclude .git "$drupal_dir/$module_path"
25+
26+
# # The rsync command in not available in itkdev/php8.1-fpm
27+
28+
# git config --global --add safe.directory /app
29+
# # Copy module files into module path
30+
# for f in $(git ls-files); do
31+
# mkdir -p "$drupal_dir/$module_path/$(dirname "$f")"
32+
# cp "$f" "$drupal_dir/$module_path/$f"
33+
# done
34+
35+
# drupal_composer config minimum-stability dev
36+
37+
# # Allow ALL plugins
38+
# # https://getcomposer.org/doc/06-config.md#allow-plugins
39+
# drupal_composer config --no-plugins allow-plugins true
40+
41+
# drupal_composer require wikimedia/composer-merge-plugin
42+
# drupal_composer config extra.merge-plugin.include "$module_path/composer.json"
43+
# # https://www.drupal.org/project/drupal/issues/3220043#comment-14845434
44+
# drupal_composer require --dev symfony/phpunit-bridge
45+
46+
# Run PHPStan
47+
(cd "$drupal_dir" && vendor/bin/phpstan --configuration="$module_path/phpstan.neon")

src/Exception/RuntimeException.php

+4-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
namespace Drupal\os2web_datalookup\Exception;
44

5-
class RuntimeException extends \RuntimeException
6-
{
5+
/**
6+
*
7+
*/
8+
class RuntimeException extends \RuntimeException {
79

810
}

src/Plugin/os2web/DataLookup/DataLookupBase.php

+16-13
Original file line numberDiff line numberDiff line change
@@ -44,20 +44,25 @@ abstract class DataLookupBase extends PluginBase implements ContainerFactoryPlug
4444
/**
4545
* {@inheritdoc}
4646
*/
47-
public function __construct(array $configuration, $plugin_id, $plugin_definition,
47+
public function __construct(
48+
array $configuration,
49+
$plugin_id,
50+
$plugin_definition,
4851
protected KeyRepositoryInterface $keyRepository,
49-
protected FileSystem $fileSystem
52+
protected FileSystem $fileSystem,
5053
) {
5154
parent::__construct($configuration, $plugin_id, $plugin_definition);
5255
$this->setConfiguration($configuration);
5356
$this->init();
5457
}
5558

56-
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition)
57-
{
58-
/** @var KeyRepositoryInterface $keyRepository */
59+
/**
60+
*
61+
*/
62+
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
63+
/** @var \Drupal\key\KeyRepositoryInterface $keyRepository */
5964
$keyRepository = $container->get('key.repository');
60-
/** @var FileSystem $fileSystem */
65+
/** @var \Drupal\Core\File\FileSystem $fileSystem */
6166
$fileSystem = $container->get('file_system');
6267

6368
return new static(
@@ -72,11 +77,10 @@ public static function create(ContainerInterface $container, array $configuratio
7277
/**
7378
* Plugin init method.
7479
*/
75-
protected function init()
76-
{
80+
protected function init() {
7781
}
7882

79-
/**
83+
/**
8084
* {@inheritdoc}
8185
*/
8286
public function label() {
@@ -152,7 +156,7 @@ protected function getCertificate(): string {
152156
* The local certificate path.
153157
*/
154158
protected function createLocalCertPath(): string {
155-
$this->localCertPath = $this->fileSystem->getTempDirectory().'/'.uniqid('os2web_datalookup_local_cert_');
159+
$this->localCertPath = $this->fileSystem->getTempDirectory() . '/' . uniqid('os2web_datalookup_local_cert_');
156160

157161
return $this->localCertPath;
158162
}
@@ -161,10 +165,9 @@ protected function createLocalCertPath(): string {
161165
* Write certificate to temporary certificate file.
162166
*
163167
* @return string
164-
* The local certificate path.
168+
* The local certificate path.
165169
*/
166-
protected function writeCertificateToFile(): string
167-
{
170+
protected function writeCertificateToFile(): string {
168171
// Write certificate to local_cert location.
169172
$certificate = $this->getCertificate();
170173
$localCertPath = $this->localCertPath;

0 commit comments

Comments
 (0)