-
-
Notifications
You must be signed in to change notification settings - Fork 209
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
Adding test for $dynamicRef contain json pointer #730
Conversation
@gregsdennis can you please review this PR |
@MeastroZI please don't tag us pre-emptively. We're notified of the PR merely by you opening it. If something is urgent, you're welcome to contact us in Slack. |
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 don't think we need this.
{ | ||
"description" : "json pointer in $dynamicRef", | ||
"schema":{ | ||
"$schema":"https://json-schema.org/draft/2020-12/schema", |
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.
We already have this covered. See line 715.
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.
Although the test on 715 is a full URI with fragment; so this does ensure you are picking up the correct root when only a fragment is supplied. I would say the description is wrong, but it is not necessarily a duplicate test. That said, it feels more like one of those extended "implementation" tests (e.g. we don't have tests for syntactically borked URIs because the baseline assumption is that you are following the underlying rules for URIs, numbers etc.)
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 thing this is a duplicate. See line 134.
I'm really sorry, Greg Dennis I opened this PR because I found issue #600 and wanted to help out in the repo. I've been trying to be more active here since I couldn't contribute much before the application deadline due to joining late and working on qualification tasks. But in the rush, I forgot that I might be making things tougher for the mentors 😥 . Sorry about that. Matthew Adams and Greg Dennis |
As the test proposed by this PR seems to be a duplicate, I am closing it. However, clarification on issue #600 would be appreciated. If the issue has been resolved, it can be closed. Thank you |
Test for the $dynamicRef contain json pointer, addressing issue #600 .