-
Notifications
You must be signed in to change notification settings - Fork 4
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
GH-135: Follow URI scheme rules #136
base: main
Are you sure you want to change the base?
Conversation
spec/index.html
Outdated
<p>IRIs in the RDF abstract syntax MUST be <a data-cite="RFC3986#section-5">resolved</a> | ||
per [[RFC3986]], | ||
and MAY contain a fragment identifier.</p> | ||
per [[RFC3986]], and MAY contain a fragment identifier. IRIs SHOULD |
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.
As suggested in my review,
I would suggest to rephrase this as follows
(sorry, I can not use the suggestion mode, because the first line is not part of the diff)
IRIs in the RDF abstract syntax MAY NOT be relative references, and MUST be resolved per [RFC3986]. They MAY contain a fragment identifier.
I like the addition about following the rules defined by the scheme.
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.
Added. MAY NOT
is not in RFC 2119; "MAY" means optional. I think you meant a "MUST NOT" strength condition.
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 a bit concerned about saying that special rules without listing other important schemes, as this will lead to interoperability issues.
Which schemes did you have in mind? I picked HTTP (important) and DID (W3C) for the note. URNs are complicated (there are rules per namespace).
Some schemes don't exist! |
those certainly make sense given the context, but we make a broad statement that could include obscure schemes, or those yet to be released. White-listing specific schemes (http, https, and did, roof example) would enhance interoperability. |
Is the note text enough?
I have changed it to "HTTP/HTTPS" An advice paragraph somewhere to use specific schemes might be nice (but outside this PR). Aside: ReSpec has used not used the more usual RFC reference -- https://datatracker.ietf.org/doc/html/rfc7230. |
I think @gkellogg 's concern could be addressed in the note, by rephrasing it:
|
I think @pchampin’s suggested wording is okay. The SHOULD makes testing a bit challenging, though. Typically, we do not test SHOULD/MAY behavior. |
Note updated. If they don't recognize a scheme, it isn't an option to ignore - they must! For the testing, my ideal is having "SHOULD" is there because (1) this is new, not in RDF 1.1 (2) the "rules" aren't always cut-and-dried. |
I will squash the PR to one commit before merge; for now, the PR is keeping each change as a commit. |
This closes #135.
This PR is close to the changes in #131 and #132 and will probably need adjusting after they are merged.
Preview | Diff