Skip to content

Conversation

@mlussi90
Copy link
Contributor

Q A
Documentation no
Bugfix no
BC Break no
New Feature yes
RFC no
QA no

Description

#98 Adding a ToString Filter according to the requirements from @vaclavvanik

Signed-off-by: Markus Lussi <info@m-lussi.de>
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks @mlussi90

The ToString filter will need to be added to the default set of filters in FilterPluginManager - additionally, the v2 docs will need updating 👍

…, add condition for stringable, fix psalm return type definition

Signed-off-by: Markus Lussi <info@m-lussi.de>
@mlussi90
Copy link
Contributor Author

i tried to implement all your requested changes, can you take a look?

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks @mlussi90 - Nearly there…

You can fix coding style problems by running vendor/bin/phpcbf and psalm issues with the tests can be silenced with vendor/bin/psalm --set-baseline=psalm-baseline.xml

Signed-off-by: Markus Lussi <info@m-lussi.de>
Signed-off-by: Markus Lussi <info@m-lussi.de>
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thank you @mlussi90

In order to get CI green for Psalm, you'll need to refresh the baseline - this sort of thing can happen, particularly on Mac, so it's not your fault!

Try running vendor/bin/psalm --update-baseline --no-cache --threads=1 and verify there are no more problems with vendor/bin/psalm --no-cache --threads=1, then commit the altered baseline.

The BC check failure can be ignored - it's complaining about the altered default value for plugin manager factories.

Signed-off-by: Markus Lussi <info@m-lussi.de>
@mlussi90
Copy link
Contributor Author

@gsteel thank you very much for your help, the checks are looking better now i think

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks @mlussi90

I'll merge and release shortly, once I've got the docs organised 👍

Signed-off-by: George Steel <george@net-glue.co.uk>
@gsteel gsteel changed the title implement new ToString filter, added unittests for the new filter Add ToString filter Oct 31, 2024
@gsteel gsteel linked an issue Oct 31, 2024 that may be closed by this pull request
@gsteel gsteel merged commit 515f081 into laminas:2.39.x Oct 31, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ToString filter missing?

2 participants