-
Notifications
You must be signed in to change notification settings - Fork 27
fix/ref rfc 9535 #215
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
base: main
Are you sure you want to change the base?
fix/ref rfc 9535 #215
Conversation
Signed-off-by: Vincent Biret <vincentbiret@hotmail.com>
Signed-off-by: Vincent Biret <vincentbiret@hotmail.com>
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 think I'd prefer some of this content move to the README.
|
|
||
| Users SHALL refrain from using additional capabilities offered by the tool's JSONPath implementation to avoid any interoperability issues with their Overlay Documents. | ||
|
|
||
| In case the tool being use is not fully compliant with RFC9535, users MIGHT have to update some JSONPath query expressions to accommodate for the implementation in use. |
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'm not sure this statement is needed / appropriate. When a tool doesn't comply with this spec, I think it would be better to recommend that users find a compliant tool than adjust their usage to fit the non-compliant tool.
|
|
||
| ### Example of updates to JSONPath | ||
|
|
||
| This example JSONPath query expression: | ||
|
|
||
| ```jsonpath | ||
| $.paths.*.get.parameters[?@.name=='filter' && @.in=='query'] | ||
| ``` | ||
|
|
||
| Might require additional optional parenthesis with some implementations like so: | ||
|
|
||
| ```jsonpath | ||
| $.paths.*.get.parameters[?(@.name=='filter' && @.in=='query)'] | ||
| ``` | ||
|
|
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 too seems inappropriate for the spec, though I agree it is useful information. Could we put this in the README instead, as a subsection of "Tools that Support Overlays"?
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 also considered putting this as an appendix in the beginning of writing this. But since I have parts that are normative above, I put it in the full specification. Maybe a better approach would be to keep the informational parts and the example as an appendix, and get rid of the normative parts of this section?
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.
+1 with a few minor suggestions
|
|
||
| In case the tool being use is not fully compliant with RFC9535, users MIGHT have to update some JSONPath query expressions to accommodate for the implementation in use. | ||
|
|
||
| ### Example of updates to JSONPath |
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 would call this a restriction and not an "update" 😎
This whole section should rather go into the README, as proposed by @mikekistler.
Co-authored-by: Ralf Handl <ralf.handl@gmail.com>
fixes #88