PHP Config - Add Profile filters by loic425 · Pull Request #1556 · Behat/Behat

loic425 · 2024-12-03T11:45:59Z

Referenced issue #1539

use Behat\Config\Config;
use Behat\Config\Profile;
use Behat\Config\Filter\NameFilter;
use Behat\Config\Filter\TagFilter;

return (new Config())
    ->withProfile(
        (new Profile('default'))
            ->withFilter(new NameFilter('name1'))
            ->withFilter(new TagFilter('tag2'))
  )
;

loic425 · 2024-12-04T07:52:41Z

@acoulton @carlos-granados
I think this is now ready to review. I prefer my today implementation.

loic425 · 2024-12-04T07:54:26Z

src/Behat/Config/Gherkin/Filter/FilterInterface.php

+{
+    public function name(): string;
+
+    public function value(): mixed;


the interface value returns a mixed but the other current filters return a string.
I'm not aware if some other filters can have other value types. So, I've specified mixed on the interface but tell me if I am wrong.

Right now the types of filters is limited and there is no way to add new filters, so I would say this can be a string

carlos-granados

Some comments

carlos-granados · 2024-12-04T08:20:01Z

src/Behat/Config/Gherkin/Filter/FilterInterface.php

+
+declare(strict_types=1);
+
+namespace Behat\Config\Gherkin\Filter;


The filters can also be applied on the suite level, so perhaps it would be better to remove the Gherkin part of the namespace? I know that the files that implement the filtering are in this Gherkin namespace but I don't think that forces us to use the same here

carlos-granados · 2024-12-04T08:20:09Z

src/Behat/Config/Gherkin/Filter/FilterInterface.php

+{
+    public function name(): string;
+
+    public function value(): mixed;


Right now the types of filters is limited and there is no way to add new filters, so I would say this can be a string

carlos-granados · 2024-12-04T08:23:03Z

src/Behat/Config/Gherkin/SimpleFilter.php

@@ -0,0 +1,26 @@
+<?php


I don't think this is needed. If you want to create one of the existing filters just use the corresponding class. And any new filters that you add will not be handled, Behat will throw an exception

Unless perhaps all previous filters extend this SimpleFilter with most of the functionality delegated to it

Yes, I forgot to remove it. I think we can start simple without it at all.

I've just removed it.

carlos-granados

LGTM, thanks @loic425

acoulton

Great, thanks @loic425

PHP Config - Add Gherkin filters

8d174d6

loic425 force-pushed the php-config-gherkin-filters branch from 3f7b9c3 to 8d174d6 Compare December 4, 2024 07:50

loic425 commented Dec 4, 2024

View reviewed changes

carlos-granados reviewed Dec 4, 2024

View reviewed changes

loic425 added 2 commits December 4, 2024 10:45

Remove Gherkin namespace

0bbbdcf

Restrict value from filter interface to be a string

e55b5fa

carlos-granados approved these changes Dec 4, 2024

View reviewed changes

acoulton approved these changes Dec 4, 2024

View reviewed changes

acoulton merged commit f2d2379 into Behat:master Dec 4, 2024

loic425 deleted the php-config-gherkin-filters branch December 4, 2024 13:09


		declare(strict_types=1);

		namespace Behat\Config\Gherkin\Filter;

Uh oh!

Comments

Conversation

loic425 commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

loic425 commented Dec 4, 2024

Uh oh!

loic425 Dec 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-granados left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlos-granados left a comment

Choose a reason for hiding this comment

Uh oh!

acoulton left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

loic425 commented Dec 3, 2024 •

edited

Loading

loic425 Dec 4, 2024 •

edited

Loading