chore: remove twitter #2255
Merged
chore: remove twitter #2255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mroderick
commented
Aug 6, 2025
| %meta{ name: 'twitter:data1', value: "#{workshop.host.name}, #{@host_address}" } | ||
| %meta{ name: 'twitter:label2', value: "When"} | ||
| %meta{ name: 'twitter:data2', value: "#{humanize_date(workshop.date_and_time, with_time: true)}" } | ||
| %meta{ property: 'og:image', content: image_url('social-summary-card-image.png') } |
There was a problem hiding this comment.
The file twitter-summary-card-image.png was renamed to social-summary-card-image.png
|
@mroderick might have to rebase on master to make that one failing test pass. I think this was the one that David updated and reverted later, as it was not passing on Github. Once the test are passing I'm happy to approve. |
mroderick
force-pushed
the
remove-twitter
branch
from
39b76b9 to
583e99e
Compare
mroderick
commented
Aug 11, 2025
| @@ -0,0 +1,7 @@ | |||
| class RemoveTwitter < ActiveRecord::Migration[7.0] | |||
There was a problem hiding this comment.
@olleolleolle do you foresee any issues with this migration?
| @@ -33,5 +33,4 @@ | |||
| - if event.organisers.any? | |||
| .d-md-flex.align-items-md-center | |||
| - event.organisers.each do |organiser| | |||
| = link_to twitter_url_for(organiser.twitter), class: 'border-0 d-inline-block mx-1' do | |||
| = image_tag(organiser.avatar(26), class: 'rounded-circle', title: organiser.full_name, alt: organiser.full_name) | |||
| = image_tag(organiser.avatar(26), class: 'rounded-circle', title: organiser.full_name, alt: organiser.full_name) | |||
There was a problem hiding this comment.
Future: perhaps there’s room for some contact link (perhaps a Show organisation page?) on this image? Not a blocker to merging this change.
olleolleolle
approved these changes
Aug 11, 2025
| @@ -0,0 +1,7 @@ | |||
| class RemoveTwitter < ActiveRecord::Migration[7.0] | |||
matyikriszta
approved these changes
Aug 11, 2025
There was a problem hiding this comment.
Good to go once the conflict is resolved. Thanks @mroderick.
mroderick
force-pushed
the
remove-twitter
branch
from
583e99e to
317030d
Compare
|
When I merge this:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes all links and references to Twitter (except for a couple of the localisation files, as I am not confident to do that correctly).
Rationale
https://twitter.com/This can be seen as quite a radical change, so I'd like to get @KimberleyCook and @matyikriszta to review the PR before we merge it.