Skip to content
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

Update PNS -> NRS and add in more info #338

wants to merge 2 commits into from


Copy link

commented Aug 7, 2019

w/r/t new data types and versioning

Update PNS -> NRS and add in more info
 w/r/t new data types and versioning

@joshuef joshuef closed this Aug 8, 2019

@joshuef joshuef reopened this Aug 8, 2019

@joshuef joshuef marked this pull request as ready for review Aug 8, 2019

@joshuef joshuef requested review from krishnaIndia and ustulation as code owners Aug 8, 2019


This comment has been minimized.

Copy link

commented Aug 13, 2019

edit: Oh, I just realized there is a new pull request to update the XOR-URL RFC, but my comment still applies.

Cool to see everything coming together!

A note: I see that the v query parameter (?v=<x>) will be used to address specific versions and examples of this are URLs like "safe://thisxorurl#somewhere?v=1". However, this specific kind of example is technically wrong. In the example the 'query' parameter isn't actually that because it is after the segment component and therefore a part of the segment.

In other words:

const url = require('url');
const url_arr = url.parse('safe://thisxorurl#somewhere?v=1');
// url_arr.hash == '#somewhere?v=1'
// url_arr.query == null

joshuef added a commit that referenced this pull request Aug 15, 2019


This comment has been minimized.

Copy link
Contributor Author

commented Aug 15, 2019

Closing this PR, as it's superseded by #343 which contains all fixes in the PR-PRs.

@joshuef joshuef closed this Aug 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
2 participants
You can’t perform that action at this time.