Skip to content

Conversation

@carlos-granados
Copy link
Contributor

When looking at the PR that added the filters to the PHP config I noticed that the narrative filter had not been added as the filters that could be run from the command line. It was also missing from one error message.
I also found that there was no test for running the role filter from the command line

This PR addresses all these issues

Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, just a question on option descriptions

)
->addOption(
'--narrative', null, InputOption::VALUE_REQUIRED,
"Only executeCall the features with actor description" . PHP_EOL .
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Only executeCall the... here intentional (I realise it's in the existing descriptions too)? On first read it looks odd, should it just be Only execute the...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just copied and pasted without paying much attention to the text, but you are right that this reads weird, updated

@carlos-granados carlos-granados force-pushed the allow-narrative-filter-on-cli branch from 5ad721b to 94cfa9a Compare December 7, 2024 21:15
Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@acoulton acoulton merged commit 6f4b78d into Behat:master Dec 9, 2024
17 checks passed
@carlos-granados carlos-granados deleted the allow-narrative-filter-on-cli branch December 9, 2024 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants