-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[JsonPath] Make the component RFC compliant #61132
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
Conversation
d1364da
to
99ff096
Compare
54281cb
to
ac36023
Compare
50fc0b2
to
9111782
Compare
9111782
to
4a68767
Compare
56993dc
to
cfe7dc3
Compare
cfe7dc3
to
25bedc6
Compare
25bedc6
to
a7a9f5b
Compare
Both comments addressed @stof. Thanks! |
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.
Quick review, I didn't comment on every line where a comment could apply, please take them as hints that may apply elsewhere
src/Symfony/Component/JsonPath/Tests/JsonPathComplianceTestSuiteTest.php
Show resolved
Hide resolved
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.
LGTM after these changes
} | ||
} | ||
} else { | ||
foreach ($left as $key => $value) { |
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.
this foreach means the order of the keys don't matter - is this covered by the test suite?
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.
Actually we can keep only this loop, the comparison is order independent (see https://datatracker.ietf.org/doc/rfc9535/ section 2.5.2.2)
Comments addressed in b2e7b34, thanks for the extensive review |
21352ba
to
0b5baee
Compare
0b5baee
to
0729c54
Compare
Thank you @alexandre-daubois. |
This PR makes the JsonPath component RFC compliant by removing all skipped tests of the compliance test suite.