Skip to content
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

Drop the initial resolve step for dynamic references #1142

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jdesrosiers
Copy link
Member

Fixes #1140

Without bookending or initial resolution, describing how dynamic references work becomes much more straightforward.

This is provided as a draft PR just to show what the change might look like if agreed upon. Any discussion about whether or not to make this change should happen in #1140.

@jdesrosiers
Copy link
Member Author

The draft-next branch has been merged and is now closed. The merge target for this PR has been changed to main. Here are the recommended steps to get your branch reabsed properly.

  1. Make sure your remote for the json-schema-org/json-schema-spec repo is up-to-date. (Example: git fetch upstream).
  2. Rebase your commits onto main. (Example: git rebase --onto upstream/main abcd123~1 (replace abcd123 with the commit hash of the first commit in your PR)).
  3. Force push the rebased branch to your fork. (Example: git push --force origin my-branch).

Comment on lines -1518 to -1520
The value of the "$dynamicRef" property MUST be a string which is a
IRI-Reference that contains a valid <xref target="anchor">plain name
fragment</xref>. Resolved against the current IRI base, it indicates
Copy link
Member

Choose a reason for hiding this comment

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

We still need the first sentence, which describes the value form.

"$dynamicAnchor" and only exhibits runtime dynamic behavior when referenced
with "$dynamicRef".
(schemas that reference themselves). The extension point is defined
with "$dynamicAnchor", and only exhibits runtime dynamic behavior when
Copy link
Member

Choose a reason for hiding this comment

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

The comma isn't necessary in the current form. If you want to include a comma, the second half of the sentence needs to be a complete sentence.

Suggested change
with "$dynamicAnchor", and only exhibits runtime dynamic behavior when
with "$dynamicAnchor", and it only exhibits runtime dynamic behavior when

Comment on lines +1514 to +1515
with "$dynamicAnchor", and only exhibits runtime dynamic behavior when
referenced with "$dynamicRef".
Copy link
Member

Choose a reason for hiding this comment

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

In order to remove the "$dynamicAnchor produces a $ref-able anchor" behavior, we need to adjust this sentence. We decided that dynamic behavior always occurs because referencing with $ref is now impossible.

We'll likely need to make changes in the $dynamicAnchor section as well.

@gregsdennis
Copy link
Member

@jdesrosiers do you want to pick this up again?

@jdesrosiers
Copy link
Member Author

Yes, this needs to get done before we can release, but I don't have the capacity to work on this at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Proposal: Remove initial resolution step for dynamic references
2 participants