Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC1957: Integration manager discovery #1957
MSC1957: Integration manager discovery #1957
Changes from 9 commits
29a9982
cc10444
dffe19b
2dcda7d
13d6368
e80753e
8897ea4
22c9692
865d3da
9e073e9
788796e
8b85fda
b6f0e8e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
ok, why a
managers
key withinm.integrations
, rather thanm.integration_managers
?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.
Future expansion is really the only answer I have here. The other thing would be vague and not thought out things like possibly decentralized integrations, whatever that means.
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.
Trying to sanity check that plural
integrations
is the right term here... For other .well-known URLs, we haveclient
andserver
, so this addsintegrations
to that set.I suppose .well-known data should answer questions like "For domain
[example.com]
, what do I need to know about the[client|server]
API?" Since we refer to this new API surface as the "integrations API" (which is implemented by an "integration manager"), it seems likeintegrations
does indeed follow the existing structure.It would be wonderful to record these naming style choices alongside the other bits of spec style.
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 is supposed to be plural here because it's referencing the integrations themselves rather than the manager - the content mentions the manager directly. Theory is that we can have future capability with this endpoint if we ever get around to decentralized bridges/integrations.
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 seems like it would be very desirable.
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.
agreed, but has bikeshed potential I want to avoid here.