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

Document threepids #1068

Merged
merged 12 commits into from
Nov 14, 2017
Merged

Document threepids #1068

merged 12 commits into from
Nov 14, 2017

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Nov 14, 2017

Adds the msisdn 3pid type and generally fleshes out what a 3pid
is and how they work.

This merges most of the work from Max Dor in #1039
with some tweaks and additions.

Adds the /msisdn' 3pid type and generally fleshes out what a 3pid
is and how they work.

This merges most of the work from Max Dor in #1039
with some tweaks and additions.
@ara4n
Copy link
Member

ara4n commented Nov 14, 2017

cc @maxidor fwiw.

@@ -33,6 +33,7 @@ targets:
files:
- appendices.rst
- appendices/base64.rst
- appendices/threepids.rst
Copy link
Member

Choose a reason for hiding this comment

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

this feels a bit weird shoved in between base64 and signing json. Any reason to put it here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I see what you mean. I've moved it to before the threat vectors stuff.

@@ -32,13 +34,13 @@ paths:
type: string
name: medium
required: true
description: The literal string "email".
description: The medium type of the 3pid. See `Appendices`_.
Copy link
Member

Choose a reason for hiding this comment

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

can you make these links more specific too?


3PID Types
----------
3PIDs represent identifiers on other namespaces that might be associated with a
Copy link
Member

Choose a reason for hiding this comment

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

can we spell this out in full? "Third-party identifiers (3PIDs) ..."

Copy link
Member Author

Choose a reason for hiding this comment

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

done

well-defined manner.

For example, for e-mail, the `medium` is 'email' and the `address` would be the
email address, eg. the string 'bob@example.com'. Since domain resolution is
Copy link
Member

Choose a reason for hiding this comment

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

probably want double-backticks here to stop them being linkified

Copy link
Member Author

Choose a reason for hiding this comment

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

done

3PID Types
----------
3PIDs represent identifiers on other namespaces that might be associated with a
particular person. They comprise a tuple of `medium` which is a string that
Copy link
Member

Choose a reason for hiding this comment

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

double backticks in RST

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

👍

needs lots of double-backtickery

----------
3PIDs represent identifiers on other namespaces that might be associated with a
particular person. They comprise a tuple of `medium` which is a string that
identifies the namespace in which the identifier exists and an `address`: a
Copy link
Member

Choose a reason for hiding this comment

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

suggest a comma before "and"

Copy link
Member Author

Choose a reason for hiding this comment

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

done

particular person. They comprise a tuple of `medium` which is a string that
identifies the namespace in which the identifier exists and an `address`: a
string representing the identifier in that namespace. This must be a canonical
form of the identifier, ie. if multiple strings could represent the same
Copy link
Member

Choose a reason for hiding this comment

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

i.e., and normally we italicise it because it's like foreign innit

*i.e.*

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm apart from nits

might be associated with a particular person. They comprise a tuple of ``medium``
which is a string that identifies the namespace in which the identifier exists,
and an ``address``: a string representing the identifier in that namespace. This
must be a canonical form of the identifier, *ie.* if multiple strings could
Copy link
Member

Choose a reason for hiding this comment

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

still needs a . in the middle please

address, in a well-defined manner.

For example, for e-mail, the ``medium`` is 'email' and the ``address`` would be the
email address, eg. the string ``bob@example.com``. Since domain resolution is
Copy link
Member

Choose a reason for hiding this comment

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

*e.g.*

Medium: ``email``

Represents E-Mail addresses. The ``address`` is the raw email address in
user@domain form with the domain in lowercase. It must not contain other text
Copy link
Member

Choose a reason for hiding this comment

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

backticks on user@domain


For example, for e-mail, the ``medium`` is 'email' and the ``address`` would be the
email address, eg. the string ``bob@example.com``. Since domain resolution is
case-insensitive, the email address ``bob@Example.com`` also has a 3PID address
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand this sentence, but it may be better to just land it and worry about clarifying it later.

Copy link
Member Author

Choose a reason for hiding this comment

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

have added an attempt to make it clearer

@richvdh richvdh assigned dbkr and unassigned richvdh Nov 14, 2017
@dbkr dbkr merged commit d728e67 into master Nov 14, 2017
@maxidorius
Copy link
Contributor

Awesome! thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants