Skip to content

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

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

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

Conversation

@uhoreg

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

Copy link
Copy Markdown
Member

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. Process state. proposal-in-review labels Dec 21, 2021
@Half-Shot

Half-Shot commented Dec 21, 2021

Copy link
Copy Markdown
Contributor

+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 A-Client Server Client-Server API A-Encryption E2EE labels Dec 21, 2021
@turt2live

turt2live commented Dec 21, 2021

Copy link
Copy Markdown
Member

@mscbot fcp merge

@mscbot

mscbot commented Dec 21, 2021 •
edited by anoadragon453
Loading

Copy link
Copy Markdown
Collaborator

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 Process state. proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the FCP. Process state. and removed proposal-in-review labels Dec 21, 2021
@uhoreg uhoreg removed the A-Encryption E2EE label Dec 21, 2021
@turt2live

turt2live commented Dec 21, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

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
Comment thread proposals/3582-remove-room-feedback.md
@anoadragon453

anoadragon453 commented Feb 15, 2022

Copy link
Copy Markdown
Member

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

mscbot commented Feb 15, 2022

Copy link
Copy Markdown
Collaborator

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

@mscbot mscbot added final-comment-period Process state to accept, reject, or postpone an MSC. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the FCP. Process state. labels Feb 15, 2022
@mscbot

mscbot commented Feb 20, 2022

Copy link
Copy Markdown
Collaborator

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

@mscbot mscbot added finished-final-comment-period FCP has finished. Process state. and removed disposition-merge Process state. final-comment-period Process state to accept, reject, or postpone an MSC. labels Feb 20, 2022
@turt2live turt2live merged commit e9fed10 into main Feb 20, 2022
@turt2live turt2live added spec-pr-missing MSC is accepted, but missing spec PR. Process state. and removed finished-final-comment-period FCP has finished. Process state. labels Feb 20, 2022
@turt2live turt2live mentioned this pull request May 5, 2022
Merged
@turt2live

turt2live commented May 5, 2022

Copy link
Copy Markdown
Member

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 MSC is accepted, but missing spec PR. Process state. labels May 5, 2022
@turt2live

turt2live commented May 5, 2022

Copy link
Copy Markdown
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! Process state. 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

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

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