Skip to content

MSC3582: Remove m.room.message.feedback#3582

Merged
turt2live merged 2 commits intomainfrom
uhoreg/remove_room_message_feedback
Feb 20, 2022
Merged

MSC3582: Remove m.room.message.feedback#3582
turt2live merged 2 commits intomainfrom
uhoreg/remove_room_message_feedback

Conversation

@uhoreg
Copy link
Copy Markdown
Member

@uhoreg uhoreg commented Dec 21, 2021 •
edited by turt2live
Loading

Rendered

I don't think anything implements this event, so I guess lack of implementation of the event is proof of implementation?

Fixes https://github.com/matrix-org/matrix-doc/issues/3318

Preview: https://pr3582--matrix-org-previews.netlify.app

@uhoreg uhoreg changed the title MSCxxxx: Remove m.room.message.feedback MSC3582: Remove m.room.message.feedback Dec 21, 2021
@uhoreg uhoreg added kind:maintenance MSC which clarifies/updates existing spec proposal A matrix spec change proposal proposal-in-review labels Dec 21, 2021
@Half-Shot
Copy link
Copy Markdown
Contributor

Half-Shot commented Dec 21, 2021

+1 I've never seen this as long as I've been Matrix-ing, so I think it would be fine to remove.

@turt2live turt2live added client-server Client-Server API e2e labels Dec 21, 2021
@turt2live
Copy link
Copy Markdown
Member

turt2live commented Dec 21, 2021

@mscbot fcp merge

@mscbot
Copy link
Copy Markdown
Collaborator

mscbot commented Dec 21, 2021 •
edited by anoadragon453
Loading

Team member @turt2live has proposed to merge this. The next step is review by the rest of the tagged people:

Once at least 75% of reviewers approve (and there are no outstanding concerns), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for information about what commands tagged team members can give me.

@mscbot mscbot added disposition-merge proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. and removed proposal-in-review labels Dec 21, 2021
@uhoreg uhoreg removed the e2e label Dec 21, 2021
@turt2live
Copy link
Copy Markdown
Member

turt2live commented Dec 21, 2021 •
edited
Loading

fwiw only implementation proof I can find of this event ever existing is autogenerated code. I don't think that counts as used.

deepbluev7
deepbluev7 reviewed Dec 22, 2021
View reviewed changes
@anoadragon453
Copy link
Copy Markdown
Member

anoadragon453 commented Feb 15, 2022

Searching across all of GitHub, definitions for this event type exist in matrix-ios-sdk, matrix-android-sdk2 and ruma, but none of them actually insert this into any rooms. The only non-definition code I can find is in matrix-android-sdk2, which just preserves the field from redactions. No mention in the Famedly Matrix SDK.

Given it's use is also discouraged, I think it's pretty safe to remove it.

@mscbot
Copy link
Copy Markdown
Collaborator

mscbot commented Feb 15, 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 Feb 15, 2022
@mscbot
Copy link
Copy Markdown
Collaborator

mscbot commented Feb 20, 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 Feb 20, 2022
@turt2live turt2live merged commit e9fed10 into main Feb 20, 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 Feb 20, 2022
@turt2live turt2live mentioned this pull request May 5, 2022
Merged
@turt2live
Copy link
Copy Markdown
Member

turt2live commented May 5, 2022

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

@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 May 5, 2022
@turt2live
Copy link
Copy Markdown
Member

turt2live commented May 5, 2022

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 May 5, 2022
This was referenced May 27, 2022
Closed
Closed
@zecakeh zecakeh mentioned this pull request Jun 20, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@turt2live turt2live turt2live left review comments

@KitsuneRal KitsuneRal KitsuneRal left review comments

+2 more reviewers

@deepbluev7 deepbluev7 deepbluev7 left review comments

@kevincox kevincox kevincox approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal

Projects

Spec Core Team Workflow
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@uhoreg @Half-Shot @turt2live @mscbot @anoadragon453 @kevincox @KitsuneRal @deepbluev7 @richvdh