Add gherkin_lint task #1144
Add gherkin_lint task #1144
Conversation
saidatom
force-pushed
the
update-gherkin-lint
branch
from
56ac251
to
ad8695c
Compare
composer.json
Outdated
| @@ -52,6 +52,7 @@ | |||
| "brianium/paratest": "Lets GrumPHP run PHPUnit in parallel.", | |||
| "codeception/codeception": "Lets GrumPHP run your project's full stack tests", | |||
| "consolidation/robo": "Lets GrumPHP run your automated PHP tasks.", | |||
| "dantleech/ngherkin-lint-php": "Lets GrumPHP lint your Gherkin files.", | |||
There was a problem hiding this comment.
typo? dantleech/ngherkin-lint-php
| { | ||
| $resolver = new OptionsResolver(); | ||
| $resolver->setDefaults([ | ||
| 'directory' => 'features', |
There was a problem hiding this comment.
I noticed that if the specified directory doesn't exist, it fails with:
In FeatureFinder.php line 25:
Provided path "/var/www/html/features" does not exist
lint <path>
Failed to run grumphp run --tasks=gherkin_lint: exit status 1
Is this expected ? or should we skip in such cases?
How are other tasks handling these cases?
There was a problem hiding this comment.
Yes, it's expected. For example src/Task/Gherkin.php has the same configuration, and returns the same error.
|
|
||
| $resolver->addAllowedTypes('directory', ['string']); | ||
| $resolver->addAllowedTypes('config', ['null', 'string']); | ||
| $resolver->addAllowedValues('config', [null, 'text']); |
There was a problem hiding this comment.
shouldn't config be any string? Now you are limiting it to the literal text here ?
On second notice: there doesn't seem a way to specify the configuration file. It assumes a hardcoded path instead:
- https://github.com/dantleech/gherkin-lint-php?tab=readme-ov-file#configuration
- https://github.com/dantleech/gherkin-lint-php/blob/f3277c4945261d26a3098b5e003b51d815eaed4b/bin/gherkinlint#L20-L27
- https://github.com/dantleech/gherkin-lint-php/blob/f3277c4945261d26a3098b5e003b51d815eaed4b/src/Command/LintCommand.php#L27-L32
So I'dd say its better to remove the config option alltogether.
|
|
||
| public function provideExternalTaskRuns(): iterable | ||
| { | ||
| yield 'defaults' => [ |
There was a problem hiding this comment.
this provider should contain a test for every configurable option to make sure it behaves the correct way.
gherkin-lint-phpmight be easier to set up and configure, especially if you only need basic Gherkin linting.New Task Checklist:
run()method readable?run()method using the configuration correctly?