Skip to content

Conversation

@drealecs
Copy link
Contributor

suppress some psalm errors that are actually bugs in current psalm implementation or are related to deserialization workaround

…plementation or are related to deserialization workaround
@drealecs drealecs force-pushed the fix_psalm_reported_issues branch from af073a6 to c55aa7f Compare February 27, 2021 06:59
Copy link
Member

@mnapoli mnapoli left a comment

Choose a reason for hiding this comment

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

Thanks for dealing with these issues!

Looks good to me, I added 2 questions inline.

*
* @psalm-pure
* @psalm-assert T $value
* @param mixed $value
Copy link
Member

Choose a reason for hiding this comment

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

Why add this parameter docblock?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a MissingParamType psalm error.
https://psalm.dev/docs/running_psalm/issues/MissingParamType/

* @psalm-pure
* @psalm-assert T $value
* @param mixed $value
* @return string
Copy link
Member

Choose a reason for hiding this comment

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

Same here, why add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here. The @return I guess it's just the PhpStorm complaining about.

@drealecs
Copy link
Contributor Author

drealecs commented Mar 2, 2021

Just a mention here. This is not very important to be released after merge. The changes are mostly for fixing pipeline on master.

@mnapoli
Copy link
Member

mnapoli commented Mar 2, 2021

Thanks!

@mnapoli mnapoli merged commit af4b562 into myclabs:master Mar 2, 2021
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