Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use static locations for Riot icons #4779

Merged
merged 3 commits into from
Mar 6, 2019
Merged

Use static locations for Riot icons #4779

merged 3 commits into from
Mar 6, 2019

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Mar 1, 2019

See element-hq/element-web#9009

Requires riot.im changes - see timeline Deployed

@turt2live turt2live changed the title Use a static locations for Riot icons Use static locations for Riot icons Mar 1, 2019
@codecov

This comment has been minimized.

@turt2live turt2live marked this pull request as ready for review March 5, 2019 17:33
@turt2live turt2live requested a review from a team March 5, 2019 17:33
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

Again, looks plausible, but I'd like to know why we aren't going to have the same problem again.

changelog.d/4779.misc Outdated Show resolved Hide resolved
@turt2live
Copy link
Member Author

I'd like to know why we aren't going to have the same problem again.

Up until this, riot-web and other random uploads in random directories on several domains were responsible for the various avatars. Instead of hoping that things don't change and aren't forgotten (one of the logos I replaced was a Vector logo), we now have the icons in a central place in version control in a dedicated directory. The risk at this point is forgetting to update the assets, but that's better than accidentally deleting something that another thing was using.

@richvdh
Copy link
Member

richvdh commented Mar 6, 2019

we now have the icons in a central place in version control in a dedicated directory.

ok, this certainly sounds like a sensible improvement. is there any scope for a README or something in that directory that reminds people that the URLs are used in notification emails, so to avoid breaking them if possible?

@turt2live
Copy link
Member Author

Yup, already included :)

The images contained here are used by email notifications and such - do not rename or delete them without replacing all their references in other projects (sydent, synapse, etc).

(see the riot.im PR referenced in the timeline earlier up)

Co-Authored-By: turt2live <travpc@gmail.com>
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

\o/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants