Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.x update stan #681

Closed
wants to merge 3 commits into from
Closed

2.x update stan #681

wants to merge 3 commits into from

Conversation

LordSimal
Copy link
Contributor

No description provided.

@LordSimal LordSimal added this to the 2.x milestone Nov 17, 2024
@@ -50,7 +50,7 @@ class Result implements ResultInterface
/**
* Sets the result status, identity, and failure messages
*
* @param null|array|\ArrayAccess $data The identity data
* @param null|array|\ArrayAccess|mixed $data The identity data
Copy link
Contributor Author

@LordSimal LordSimal Nov 17, 2024

Choose a reason for hiding this comment

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

These mixed types needed to be added since we check for this and manually throw an exception. Otherwise phpstan would complain, that the if() cannot be reached.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we forgot to add types to this class when updating for 5.x. I would just add proper types to the props/methods and bump the plugin version as required to minor/major.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We did not miss it, this is 2.x and therefore CakePHP 4
Or would you not update stan of the 2.x version of this plugin?

Copy link
Member

Choose a reason for hiding this comment

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

I wouldnt. Only cake5 ones IMO.

psalm-baseline.xml Outdated Show resolved Hide resolved
@LordSimal LordSimal closed this Nov 17, 2024
@LordSimal LordSimal deleted the 2.x-update-stan branch November 17, 2024 14:23
@LordSimal LordSimal changed the title update stan 2.x update stan Nov 17, 2024
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.

3 participants