-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Conversation
this is rst so name it accordingly
Move some bits of the README around. No words were changed in the making of this commit.
Fix a couple of broken links
- remove redundant "where's the spec" section: this would belong in "About matrix", but it's already there. - E2E is in beta rather than dev
Add some syntax highlighting
The demo client isn't really fit for purpose, so stop encouraging people to use it.
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.
Some minor suggestions (first time using this review thing, hopefully it works properly)
|
||
.. __: `client-user-reg`_ | ||
|
||
To get started, is easiest to use the command line to register new users:: |
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.
missing "it" before "is"? (i.e. should be "To get started, it is easiest...")
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, done
`Configuring synapse`_), and partly from a localpart you specify when you | ||
create the account. Your name will take the form of:: | ||
|
||
@localpart:my.domain.here |
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.
It should probably use "my.domain.name" instead of "my.domain.here" for consistency with the above example (also two lines below)
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.
done, thanks
provided you set the ``server_name`` to match your machine's public DNS | ||
hostname. | ||
|
||
For a more flexible conversation, you can set up a DNS SRV record. This allows |
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.
s/conversation/configuration/ ?
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!
(``tls_private_key_path`` will be ignored if ``no_tls`` is ``True``.) | ||
|
||
* In your reverse-proxy configuration, if there are other virtual hosts on the | ||
same port, make sure that Synapse is the default. |
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.
Maybe make a note that Synapse only needs /_matrix/*
forwarded, so the proxy can forward other paths in the default virtual host to another server? Not sure of the best wording.
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.
I've tried to reword this a bit.
|
||
Meanwhile, iOS and Android SDKs and clients are available from: | ||
|
||
- https://github.com/matrix-org/matrix-ios-sdk |
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.
It feels like a shame to remove the links to the official SDKs here. I know we also link to the huge project page, but saying "hey, the official SDKs for JS, iOS & Android are here - please go get started!" feels worthwhile.
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.
well, where does it end? do we include the python sdk? the react sdk? I'll add a link to the "SDKs section" of the project page, which I think will be clearer than listing the SDKs here.
|
||
Thanks for using Matrix! | ||
|
||
[1] End-to-end encryption is currently in development - see https://matrix.org/git/olm | ||
[1] End-to-end encryption is currently in beta. |
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.
This is a great place surely to link to info on e2e, which will be the next question of anyone reading it. Surely we should link to it, or at least to the beta blog post or something?
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.
yup, have linked to the matrix.org blog post.
determines the "domain" part of user-ids for users on your server: these will | ||
all be of the format ``@user:my.domain.name``. It also determines how other | ||
matrix servers will reach yours for `Federation`_. For a test configuration, | ||
set this to the hostname of your server. For a more production-ready setup, you |
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.
I think we should warn people to get the right name from the get go, given they can't rename servers.
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.
Yeah, this is a difficult one, imho. The problem with trying to get people to use domain.com
rather than mytestserver.domain.com
from the get-go is that it means they need to grok the whole SRV business from the get-go, which is probably rather a lot to grasp.
As a compromise, I'll warn here that a server name can't be changed.
also generate a set of keys for you. These keys will allow your Home Server to | ||
identify itself to other Home Servers, so don't lose or delete them. It would be | ||
wise to back them up somewhere safe. If, for whatever reason, you do need to | ||
change your Home Server's keys, you may find that other Home Servers have the | ||
old key cached. If you update the signing key, you should change the name of the | ||
key in the <server name>.signing.key file (the second word) to something different. | ||
key in the ``<server name>.signing.key`` file (the second word) to something different. |
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.
this obviously begs the question as to how keys & perspectives are validated. i suspect we should update it with a quick explanation of wherever the key validation stuff is at.
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.
I've added a link to the spec, but afaict this stuff is not well-documented.
key in the <server name>.signing.key file (the second word) to something different. | ||
key in the ``<server name>.signing.key`` file (the second word) to something different. | ||
|
||
The default configuration exposes two TCP ports: 8008 and 8448. Port 8008 is |
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.
surely we should spell out whether these are for federation, CS, or both, or what
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.
Added some words.
* Due to the way SSL certificates are managed in the Matrix federation protocol | ||
(see `spec <https://matrix.org/docs/spec/server_server/unstable.html#retrieving-server-keys>`_), | ||
Synapse needs to be configured with the path to the SSL certificate, *even if | ||
you do not terminate SSL at Synapse*. |
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.
I do not think this is true any more, since Mark's TLS changes - all synapse needs is to be told the fingerprint of the certificate, which can be passed in as a tls_certificate_fingerprint
option or something like that?
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.
well, maybe, but I know nothing about it. Can we make Mark document it here as a separate PR?
configured without TLS; it is not recommended this be exposed outside your | ||
local network. Port 8448 is configured to use TLS with a self-signed | ||
certificate. This is fine for testing with but you will almost certainly want | ||
to use another certificate for production purposes. You can do so by changing |
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.
this contradicts the doc later on, where we say that self-signed are fine. i think we should spell out better when we can and can't get away with self-signed (i.e. client v. fed, again)
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.
added some words.
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.
I'm reluctant to get too deep into the various options for different ports for federation and clients here, because I think it just adds noise that will confuse the first-time user. It's set out in more detail in the reverse-proxy section.
Matrix users is also very security-sensitive for similar reasons. | ||
Identity servers have the job of mapping email addresses and other 3rd Party | ||
IDs (3PIDs) to Matrix user IDs, as well as verifying the ownership of 3PIDs | ||
before creating that mapping. |
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.
in bold: They are not where accounts or credentials are stored - these live on home serves. Identity Servers are just for mapping 3rd party IDs to matrix IDs.
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.
(i know you say this at the end, but i think it bears reiterating)
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.
added
|
||
The source of the matrix spec lives at https://github.com/matrix-org/matrix-doc. | ||
A recent HTML snapshot of this lives at http://matrix.org/docs/spec | ||
Synapse Development |
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.
I'm unsure about bumping synapse development so low here. I almost feel we should give users the choice of either installing via pip or git (or OS package) from the outset, as it's common for people who install their own server to then want to fiddle with it a bit, and obviously a git checkout is the best platform for doing so...
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.
I disagree. The README is hard to grok as it is. I want to make it easier for people to get set up without support from us; anyone who gets that far and starts thinking about hacking on it will be able to find this.
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.
I think it might be worth it to split all the install instructions into an INSTALL file to get the README a bit more manageable.
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.
@kyrias: that's a good suggestion, but it's hard to see where to draw the line. At the end of the day, most of the README is installation instructions, so this might just lead to a massive INSTALL file. (plus we tend to link to bits of it from everywhere).
Another option might be to split more bits of it out into the docs
directory and link to them from the README.
Either way, I'd like to get this merged rather than blocking it until I have time to spend another day on it.
|
||
UPDATE users SET password_hash='$2a$12$xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx' | ||
WHERE name='@test:test.com'; | ||
|
||
Where's the spec?! |
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.
Why delete the link to the spec? This is kinda important...
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.
From the commit comment:
- remove redundant "where's the spec" section: this would belong in "About matrix", but it's already there.
It could probably bear being made clearer in "about matrix", but I'm not sure that the synapse readme needs to be a comprehensive introduction to Matrix - that job is better done elsewhere on matrix.org.
Minor updates post-review
327a759
to
9df84dd
Compare
lgtm |
Attempt to drag the README into 2016.