-
-
Notifications
You must be signed in to change notification settings - Fork 874
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
Install phpstan locally and use extensions #2606
Install phpstan locally and use extensions #2606
Conversation
198b9d1
to
37a2bf6
Compare
1b6f881
to
98cfc26
Compare
|
||
if (!isset($item['data'])) { | ||
throw new InvalidArgumentException('The JSON API document must contain a "data" key.'); | ||
throw new UnexpectedValueException('The JSON API document must contain a "data" key.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be a BC break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider it a bug fix, because we're not supposed to throw this type of exception from NormalizerInterface::normalize
in the first place.
title says |
@bendavies What do you mean? It does make the switch to phpstan-shim, which allows us to use extensions. That's very important as otherwise many problems go undetected. |
@teohhanhui where do you use the shim? i see |
@bendavies Oops. You're right. I didn't even realize that, but the intention was to use the shim. 😂😂😂 Yeah, I guess it's not actually necessary to use the shim. |
FYI here is why i'd advise against the shim: |
@bendavies That looks like a general Composer issue, but yeah, we have no need to use the shim, and could easily switch if / when the need arises. |
yes, it's a composer issue, but one that manifests if you use the shim, which is very annoying :) |
7bbfa76
to
e2874b4
Compare
Co-authored-by: soyuka <soyuka@users.noreply.github.com>
e2874b4
to
9352907
Compare
Thanks @teohhanhui ! |
Replaces #2603