Skip to content

MSC2376: Disable URL Previews #2376

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
Closed
wants to merge 6 commits into from
Closed

MSC2376: Disable URL Previews #2376

wants to merge 6 commits into from

Conversation

Sorunome
Copy link
Contributor

@Sorunome Sorunome commented Dec 3, 2019 •
edited
Loading

Rendered

This MSC provides an alternative method to MSC2385

Signed-off-by: Sorunome [email protected]

anoadragon453
anoadragon453 requested changes Dec 3, 2019
View reviewed changes
Copy link
Member

@anoadragon453 anoadragon453 left a comment

Choose a reason for hiding this comment

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

Some wording things. Functionally this MSC looks good.

dali99
dali99 reviewed Dec 3, 2019
View reviewed changes
KitsuneRal
KitsuneRal requested changes Dec 3, 2019
View reviewed changes
Copy link
Member

@KitsuneRal KitsuneRal left a comment

Choose a reason for hiding this comment

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

I'm not quite happy with HTML customisation approach - see my comments below. The described alternative looks much more reasonable to me, as long as we enforce that URLs in the no-preview list match those in the body.

@turt2live turt2live added proposal A matrix spec change proposal proposal-in-review labels Dec 3, 2019
@turt2live turt2live self-requested a review December 3, 2019 17:23
uhoreg
uhoreg reviewed Dec 3, 2019
View reviewed changes
Copy link
Member

@uhoreg uhoreg left a comment

Choose a reason for hiding this comment

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

Looks pretty good.

@Sorunome Sorunome force-pushed the soru/no-url-previews branch from 1b987de to 9211a54 Compare December 3, 2019 17:33
@Sorunome
Copy link
Contributor Author

Sorunome commented Dec 8, 2019

I'm not quite happy with HTML customisation approach - see my comments below. The described alternative looks much more reasonable to me, as long as we enforce that URLs in the no-preview list match those in the body.

Should soru make a competing MSC to this one with that other proposal?

@KitsuneRal
Copy link
Member

I'm not quite happy with HTML customisation approach - see my comments below. The described alternative looks much more reasonable to me, as long as we enforce that URLs in the no-preview list match those in the body.

Should soru make a competing MSC to this one with that other proposal?

I'd appreciate that.

@Sorunome Sorunome mentioned this pull request Dec 8, 2019
Open
@Sorunome
Copy link
Contributor Author

Sorunome commented Dec 8, 2019

I'm not quite happy with HTML customisation approach - see my comments below. The described alternative looks much more reasonable to me, as long as we enforce that URLs in the no-preview list match those in the body.

Should soru make a competing MSC to this one with that other proposal?

I'd appreciate that.

Done! #2385

turt2live
turt2live approved these changes Feb 5, 2020
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Personally I prefer this version over #2385 for its simplicity.

@turt2live turt2live added the kind:feature MSC for not-core and not-maintenance stuff label Apr 20, 2020
@hungrymonkey hungrymonkey mentioned this pull request Jul 30, 2020
Closed
@tulir tulir mentioned this pull request Dec 7, 2020
Merged
2 tasks
@tulir tulir mentioned this pull request Dec 26, 2020
Open
@Sorunome
Copy link
Contributor Author

Soru thinks she prefers #2385 over this one. Should she close this MSC or leave it open, in case others prefer this one?

@uhoreg
Copy link
Member

uhoreg commented Jan 13, 2021

If you, as the author, prefer another approach over the one proposed here, you can feel free to close this one. Since your other MSC links to this one, others can find it, and if they have strong enough reasons to prefer it and pursue it, they can always re-propose it.

@Sorunome
Copy link
Contributor Author

Alright, closing this one, then. If anyone wants to re-propose this MSC, they are free to re-use the same current proposal, to not have to write something from scratch 👍

@Sorunome Sorunome closed this Jan 14, 2021
@turt2live turt2live added abandoned A proposal where the author/shepherd is not responsive and removed proposal-in-review labels Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dali99 dali99 dali99 left review comments

@uhoreg uhoreg uhoreg left review comments

@anoadragon453 anoadragon453 anoadragon453 requested changes

@KitsuneRal KitsuneRal KitsuneRal requested changes

@turt2live turt2live turt2live approved these changes

Assignees
No one assigned
Labels
abandoned A proposal where the author/shepherd is not responsive kind:feature MSC for not-core and not-maintenance stuff proposal A matrix spec change proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Sorunome @KitsuneRal @uhoreg @dali99 @turt2live @anoadragon453