Skip to content

Conversation

@DanielBadura
Copy link
Contributor

I removed the stub logic alltogether since it was not used, added some supresses to satify pslam as it seems this is the way to go here, otherwise i would have created a baseline for them.

…uppresses which are not relevant for the test case. Add some unused supresses for the plugin itself. Update deps.
Copy link
Owner

@cspray cspray 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 for submitting this change! I had one minor request, once it gets updated I will merge this in and cut a new release.


use Attribute;

/** @psalm-suppress UnusedClass */
Copy link
Owner

@cspray cspray Jan 30, 2025

Choose a reason for hiding this comment

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

I actually believe this should be annotated as @api. This communicates the class is part of the public facing API used by installers of this lib.

Edit: And, of course, getting the build to go green :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, didnt know this annotation exist.

Regarding failing build: would drop PHP 8.0 then :)

@DanielBadura DanielBadura changed the title Allow installation of psalm 6 Allow installation of psalm 6, Drop PHP 8.0 support Feb 7, 2025
@DanielBadura
Copy link
Contributor Author

@cspray could you again have a look on this patch?

@cspray
Copy link
Owner

cspray commented Feb 11, 2025

@DanielBadura Sorry for the delay. I will review this today. Thanks!

@cspray
Copy link
Owner

cspray commented Feb 12, 2025

Thank you very much for your contribution! I'll cut a new major release soon to enable using Psalm 6.

@cspray cspray merged commit 9826c34 into cspray:main Feb 12, 2025
1 check passed
@DanielBadura DanielBadura deleted the allow-psalm-v6 branch February 13, 2025 11:39
@DanielBadura
Copy link
Contributor Author

Thanks! I don't think you would need a new major version for this since the API for the user did not change. But the choice is yours :D

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