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

Fix confusing remark about NSS divergence from the spec #184

Merged
merged 1 commit into from
Jun 17, 2019

Conversation

michielbdejong
Copy link
Contributor

Fixes #182

Copy link
Member

@acoburn acoburn left a comment

Choose a reason for hiding this comment

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

👍

@michielbdejong
Copy link
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
Contributor Author

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

@csarven
Copy link
Member

csarven commented Jun 17, 2019

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

Copy link
Contributor

@RubenVerborgh RubenVerborgh left a 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)

Copy link
Member

@kjetilk kjetilk left a comment

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
Copy link
Member

csarven commented Jun 17, 2019

+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