Skip to content

add test for arbitrary URI in $id (#797)#798

Open
sangamon wants to merge 2 commits intojson-schema-org:mainfrom
sangamon:797-taguri
Open

add test for arbitrary URI in $id (#797)#798
sangamon wants to merge 2 commits intojson-schema-org:mainfrom
sangamon:797-taguri

Conversation

@sangamon
Copy link
Contributor

naive copy of test cases for URN in $id with tag URI values for issue #797

@sangamon sangamon requested a review from a team as a code owner November 25, 2025 11:55
Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

We already have tests covering multiple non-http URI schemes including urn: and file:. Do we really need another? I don't think we want to include tests for every known URI scheme. I think it's sufficient to have one or two examples.

@sangamon sangamon changed the title add test for tag URI in $id (#797) add test for arbitrary URI in $id (#797) Nov 26, 2025
Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

Sorry, this seems to have slipped through the cracks.

I looks like the consensus in the end was to include urn:, tag:, and something arbitrary like foo:. Could you bring back your original tag: tests and keep these so we're covering all three cases.

]
},
{
"description": "simple custom URI base URI with $ref via the custom URI",
Copy link
Member

Choose a reason for hiding this comment

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

I think this description needs to be more clear about what's "custom" is the URI scheme. Here's my suggestion.

Suggested change
"description": "simple custom URI base URI with $ref via the custom URI",
"description": "simple base URI using an arbitrary URI scheme with $ref to that URI",

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants