Skip to content
This repository was archived by the owner on Apr 13, 2022. It is now read-only.

Fix confusing remark about NSS divergence from the spec - #184

Merged
kjetilk merged 1 commit into
masterfrom
fix-remark-relative-urls
Jun 17, 2019
Merged

kjetilk merged 1 commit into
masterfrom
fix-remark-relative-urls

Conversation

@michielbdejong

Copy link
Copy Markdown
Contributor

Fixes #182

@acoburn acoburn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@michielbdejong

Copy link
Copy Markdown
Contributor Author

Thanks! @solid/core can someone make @acoburn a "reviewer with write access" on this repo?

@kjetilk kjetilk added the Quick Merge pull requests that can be merged in three days label Jun 6, 2019
@michielbdejong

Copy link
Copy Markdown
Contributor Author

@kjetilk @Mitzi-Laszlo @justinwb have two approving reviews now, but need one more review from a team member.

@csarven

csarven commented Jun 17, 2019

Copy link
Copy Markdown
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.

@michielbdejong

Copy link
Copy Markdown
Contributor Author

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.

@RubenVerborgh

Copy link
Copy Markdown
Contributor

How does one get elected/awarded/removed from a core team member?

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.

@RubenVerborgh RubenVerborgh left a comment

Copy link
Copy Markdown
Contributor

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)

@kjetilk kjetilk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@kjetilk
kjetilk merged commit 934daea into master Jun 17, 2019
@csarven

csarven commented Jun 17, 2019

Copy link
Copy Markdown
Member

+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.

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

Labels

Quick Merge pull requests that can be merged in three days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve correctness and clarity of relative URLs for update subscriptions

5 participants