-
-
Notifications
You must be signed in to change notification settings - Fork 962
Remove twitter social media bias #5796
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.
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
base: master
Are you sure you want to change the base?
Remove twitter social media bias #5796
Conversation
Wanted to +1 this - talking this over at Railsconf, and the use of This PR should close rubygems/rfcs#51 as well. |
@qrush I don't think so. I don't think removing Twitter/X (without replacement) is good idea, I would rather replace it with more free-form link as described at rubygems/rfcs#51. The purpose of that RFC is not to remove social media links, but to extend them to not be bound to one platform. |
df83ce6
to
b9dcce9
Compare
@simi hi! I'd rather just not have rubygems.org worry about social media links in general, but if someone else wants to add socialize links - PRs are welcome! At the very least this PR removes twitter usernames but keeps the DB column, so it could be a future migration step. I don't think we will be missing anything huge or doing a disservice to our existing users by removing this very small link from profile pages. |
@qrush I'm often grateful for any possible contact to gem authors, to help them preserve the gem. So removal will make it much harder without replacement. Wouldn't be possible to at least rename the column to more generic one, migrate current values to Twitter/X url and leave anyone to decide what to put in there? |
User.group(:twitter_username).count.sort_by(&:last).reverse.first(5).to_h
=> {nil => 220926, "" => 6382, "opss_2020" => 296, "..." => 12, "..." => 6}
User.where("twitter_username is not null and twitter_username != ''").count
=> 7206 |
Thanks @segiddins - so that's 2.7% of users with this column set in a meaningful way. That doesn't seem like it provides a lot of value for contacting a user when you can rely on going to the gem's source / github to get more info. This PR doesn't mean we can't migrate to something else later...but it does fix the bias towards just one social media site. |
@qrush if your goal is really to just remove Twitter from RubyGems.org, there's nothing more I can contribute in this discussion. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5796 +/- ##
==========================================
- Coverage 97.19% 94.42% -2.77%
==========================================
Files 471 471
Lines 9690 9748 +58
==========================================
- Hits 9418 9205 -213
- Misses 272 543 +271 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -33,7 +33,6 @@ def fields # rubocop:disable Metrics | |||
field :email_reset, as: :boolean | |||
field :handle, as: :text | |||
field :public_email, as: :boolean | |||
field :twitter_username, as: :text, as_html: true, format_using: -> { link_to value, "https://twitter.com/#{value}", target: :_blank, rel: :noopener if value.present? } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you rename to social_link
(and follow the same for helpers ...) and remove the forma_using here (swap with validation on model side to be valid link)?
|
||
<% if user.twitter_username.present? %> | ||
<div class="flex items-center mb-4 text-b3 lg:text-b2"> | ||
<%= icon_tag("x-twitter", color: :primary, class: "w-6 text-orange mr-3") %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the icon here and just use the link in link_to as is in social_link field (if present)
@qrush @segiddins @nzukie-b btw. I'm not blocking this, feel free to merge once you're happy. I can do the other part (transferring into generic |
Issue: #5511
Talked this over with @qrush and we decided to just remove twitter entirely.