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

Updating login endpoint spec to match new identifier format #1038

Closed
wants to merge 3 commits into from
Closed

Updating login endpoint spec to match new identifier format #1038

wants to merge 3 commits into from

Conversation

maxidorius
Copy link
Contributor

@maxidorius maxidorius commented Oct 25, 2017

Followed @richvdh recommendation to fix spec for next iteration

Signed-off-by: Max Dor @ Kamax.io (no email to avoid spam)

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@maxidorius
Copy link
Contributor Author

Signed-off-by: Maxime Dor @ Kamax.io (no email to avoid spam)

@maxidorius
Copy link
Contributor Author

@richvdh What can I do or who can I ping to move this forward?

@richvdh
Copy link
Member

richvdh commented Oct 26, 2017

oh, me.

Thanks very much for looking at this. I'll take a look soon but spec work has taken up a lot of time this week and I need to get some other code written to meet a deadline.

@richvdh richvdh self-assigned this Oct 26, 2017
@richvdh
Copy link
Member

richvdh commented Oct 27, 2017

@matrixbot: test this please

@richvdh
Copy link
Member

richvdh commented Nov 6, 2017

Sadly I think we're going to have to document the old format as something that servers have to support as well :/.

Also: please could you add a changelog entry?

@richvdh richvdh removed their assignment Nov 7, 2017
@maxidorius
Copy link
Contributor Author

maxidorius commented Nov 8, 2017

@richvdh not sure I follow: do you want me to redo this PR to include the old format (which was never used AFAIK) and the new format side-by-side? This PR is about documenting the existing behavior.

What can I do to only document the current behavior of things?

@richvdh
Copy link
Member

richvdh commented Nov 9, 2017

The old format was used by Riot for a long time, and is still used by other clients (for example, libqmatrixclient uses it). It still works fine for the case of email identifiers; of course for other identifiers you need to use the new format.

One of the principles behind the C-S versioning scheme is that we try hard to avoid changes that are going to break clients. The C-S Spec is the thing that documents what we expect servers to support, and in short I expect servers claiming to implement the r0 api to support the old format - though we can certainly flag it as deprecated as far as clients are concerned.

So: yes I'd like you to include both the new and old format. I'm not exactly sure what form that ought to take - it doesn't necessarily need to combine both together, provided the spec ends up documenting the old format as well as the new one so that servers can support clients like libqmatrixclient.

@maxidorius
Copy link
Contributor Author

maxidorius commented Nov 9, 2017

Ah I see, totally missed the part where this is actually still in use. I believed it was simply never true as I never saw the old format being used.
I'll think of a way to combine both, provide a commit and get back to you for review!

@maxidorius
Copy link
Contributor Author

As I'm focusing on documenting the current behavior, I'll close this for now and revisit later.
FYI, as-is code has been added into https://github.com/eventually-matrix/matrix-doc/commit/c07be4487b54a5dbde7feb1fb9d8c9a690e9327f instead.

@maxidorius maxidorius closed this Nov 10, 2017
RiotTranslateBot pushed a commit to RiotTranslateBot/matrix-doc that referenced this pull request Aug 22, 2023
…example

Fix origin server name in S2S Request Authentication example.
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