-
Notifications
You must be signed in to change notification settings - Fork 103
Fix confusing remark about NSS divergence from the spec #184
Conversation
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.
👍
Thanks! @solid/core can someone make @acoburn a "reviewer with write access" on this repo? |
@kjetilk @Mitzi-Laszlo @justinwb have two approving reviews now, but need one more review from a team member. |
How does one get elected/awarded/removed from a core team member? What are the qualifications? FWIW, as a non "core" team member, the proposed update looks good to me. |
The way we work now is we do expert review as usual (so that's Ruben, Aaron, and you, thanks!), but then all spec edits additionally need sign-off by 2 out of 5 Solid Team members, they are Tim, Mitzi, Ruben, Justin and Kjetil. That extra rule was added here in the hope that this double review (experts + team members) helps avoid situations like the one this PR is actually reverting. In this case, it has slowed down the revert, but in general the intent is that there are more eyes on all spec PRs before they get merge, which I think is a good thing. |
In addition to what @michielbdejong said, this is a temporary thing until there's a better solution, was just the quickest I could do to avoid the #181 problem. |
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.
Fine with me, but would have preferred MUST be absolute URIs
(given that SHOULD has another meaning in https://tools.ietf.org/html/rfc2119)
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.
LGTM
+1 to Ruben's suggestion to use MUST. If this is supposed to be prescriptive, it might as well be explicit. This is especially where errors/faults are not described or what happens if 1) non relative URIs or 2) non HTTP URIs are used. So, unless I'm overlooking something, the spec should say MUST HTTP URI. |
Fixes #182