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

server-server-api: Fix grammar #1665

Merged
merged 4 commits into from Oct 25, 2023
Merged

server-server-api: Fix grammar #1665

merged 4 commits into from Oct 25, 2023

Conversation

progval
Copy link
Contributor

@progval progval commented Oct 20, 2023

@progval progval requested a review from a team as a code owner October 20, 2023 19:42
@@ -187,7 +187,7 @@ to send. The process overall is as follows:
for `<hostname>`.

6. If the `/.well-known` request returned an error response, and the
SRV record was not found, an IP address is resolved using CNAME, AAAA and A
SRV records were not found, an IP address is resolved using CNAME, AAAA and A
Copy link
Contributor

@MTRNord MTRNord Oct 20, 2023

Choose a reason for hiding this comment

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

Isn't matrix set up in a way where there is only one SRV record at any given time? (Though on a quick search I didn't see anything not allowing multiple)

Copy link
Member

Choose a reason for hiding this comment

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

Standard SRV resolution allows for multiple records, and Matrix never forbade it. But also, steps 4 and 5 refer to different SRV records (_matrix-fed._tcp.<hostname> and _matrix._tcp.<hostname>), so it really should be plural.

On the other hand "the SRV records were not found" sounds like SRV records exist but could not be found for whatever reason. So maybe something like "... and no SRV records are present ...", or "... and no SRV records are found ..." might be better?

Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

Thanks!

@uhoreg uhoreg merged commit 7c19500 into matrix-org:main Oct 25, 2023
12 checks passed
@zecakeh zecakeh mentioned this pull request Nov 30, 2023
17 tasks
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

3 participants