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.

Sign up for GitHub

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

Jump to bottom

MSC3267: Reference relations #3267

Merged
merged 7 commits into from
Jul 17, 2022
Merged

MSC3267: Reference relations #3267

merged 7 commits into from
Jul 17, 2022

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Jul 5, 2021 •
edited by turt2live
Loading

@bwindels bwindels changed the title MSCXXXX: Reference relations MSC 3267: Reference relations Jul 5, 2021
@bwindels bwindels mentioned this pull request Jul 5, 2021
Merged
@uhoreg uhoreg added kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal proposal-in-review labels Jul 5, 2021
@uhoreg
Copy link
Member

uhoreg commented Jul 5, 2021

Tagged as needs-implementation even though the verification use-case in implemented, since the non-verification use-case is not yet implemented

@jryans jryans changed the title MSC 3267: Reference relations MSC3267: Reference relations Jul 7, 2021
This was referenced Oct 14, 2021
Merged
Open
@turt2live turt2live added client-server Client-Server API and removed proposal-in-review labels Nov 24, 2021
@lukasdenk lukasdenk mentioned this pull request Jan 3, 2022
Closed
@zecakeh zecakeh mentioned this pull request Mar 28, 2022
Closed
@turt2live turt2live mentioned this pull request Apr 5, 2022
Open
@zecakeh zecakeh mentioned this pull request May 17, 2022
Closed
@jplatte
Copy link
Contributor

jplatte commented May 17, 2022

Given that m.references has already found its way into the spec for encrypted events and threads have gone a different direction if I'm not getting things mixed up, should this MSC be closed?

@turt2live
Copy link
Member

(please use threads to receive in-depth/any replies)

The way m.reference has made it into the spec currently doesn't require this MSC, but it looks super strange indeed. We should fast track this to FCP, I guess.

@turt2live
Copy link
Member

going to use the key verification framework as an implementation proof for this - https://spec.matrix.org/v1.2/client-server-api/#key-verification-framework

@turt2live turt2live removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label May 20, 2022
@turt2live
Copy link
Member

@bwindels apologies, I've taken this over in an effort to get it through (finally).

@turt2live turt2live marked this pull request as ready for review May 20, 2022 01:53
anoadragon453
anoadragon453 approved these changes Jul 5, 2022
View reviewed changes
proposals/3267-reference-relations.md
for events to make a reference to another event.

A `rel_type` of `m.reference` is defined as a generic way to associate an
event with another event. As a bundle, `m.reference` relations appear as
Copy link
Member

Choose a reason for hiding this comment

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

It would be useful to link to MSC2675 here, which defines bundling related events.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, we can probably just remove the sentences about server-side aggregation given the information is repeated below under the "Server aggregation" header?

proposals/3267-reference-relations.md
Comment on lines +9 to +13
A `rel_type` of `m.reference` is defined as a generic way to associate an
event with another event. As a bundle, `m.reference` relations appear as
an object with a single `chunk` field. The `chunk` is an array of objects
with a single `event_id` field for all the child events which `m.reference`
the parent.
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we can just remove the word "all" in this sentence to eliminate any subtle confusion.

richvdh
richvdh reviewed Jul 12, 2022
View reviewed changes
proposals/3267-reference-relations.md

## Server aggregation

[MSC2674](https://github.com/matrix-org/matrix-doc/pull/2674) states
Copy link
Member

Choose a reason for hiding this comment

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

I think this is MSC2675 now?

@mscbot
Copy link
Collaborator

mscbot commented Jul 12, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Jul 12, 2022
@mscbot
Copy link
Collaborator

mscbot commented Jul 17, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Jul 17, 2022
@turt2live turt2live merged commit 3f0ba9c into old_master Jul 17, 2022
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Jul 17, 2022
turt2live added a commit that referenced this pull request Jul 17, 2022
@bwindels @turt2live @anoadragon453
MSC3267: Reference relations (#3267)
3a05bd6 
* initial draft of reference relations msc

* change MSC number

* Apply formatting

* Convert to point at present rather than ideal

* Clarify that multiple relations is a thing we don't have and won't fix here

* Fix wording to match reality, again

* fix a typo

Co-authored-by: Travis Ralston <[email protected]>
Co-authored-by: Andrew Morgan <[email protected]>
@turt2live turt2live self-assigned this Aug 3, 2022
turt2live added a commit to matrix-org/matrix-spec that referenced this pull request Aug 9, 2022
@turt2live
Spec reference relationships
22a11b3 
MSC: matrix-org/matrix-spec-proposals#3267
@turt2live turt2live mentioned this pull request Aug 9, 2022
Merged
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1206

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Aug 9, 2022
turt2live added a commit to matrix-org/matrix-spec that referenced this pull request Oct 18, 2022
@turt2live @richvdh
Spec reference relationships (#1206)
b2cc836 
* Spec reference relationships

MSC: matrix-org/matrix-spec-proposals#3267

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <[email protected]>

* Edits per code review

Co-authored-by: Richard van der Hoff <[email protected]>
@turt2live
Copy link
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Oct 18, 2022
@clokep clokep mentioned this pull request Nov 18, 2022
Closed
2 tasks
@zecakeh zecakeh mentioned this pull request Nov 25, 2022
Merged
clokep pushed a commit to clokep/matrix-spec that referenced this pull request May 3, 2023
@turt2live @richvdh @clokep
Spec reference relationships (matrix-org#1206)
b15e80b 
* Spec reference relationships

MSC: matrix-org/matrix-spec-proposals#3267

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <[email protected]>

* Edits per code review

Co-authored-by: Richard van der Hoff <[email protected]>
@S7evinK S7evinK mentioned this pull request Oct 2, 2023
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepbluev7 deepbluev7 deepbluev7 left review comments

@richvdh richvdh richvdh left review comments

@turt2live turt2live turt2live left review comments

@grinapo grinapo grinapo left review comments

@clokep clokep clokep left review comments

@anoadragon453 anoadragon453 anoadragon453 approved these changes

@KitsuneRal KitsuneRal Awaiting requested review from KitsuneRal

Assignees

@turt2live turt2live

Labels
client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Spec Core Team Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.