Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gem adoption #1842

Open
wants to merge 17 commits into
base: master
from

Conversation

@sonalkr132
Copy link
Member

sonalkr132 commented Dec 1, 2018

Related: #725
Adoption aims to facilitate gem ownership transfer between users, whether requested by gem owner or any rubygems.org user. ownership transfer is already possibly by gem owner -a/r command, however to quote bf4:

Being able to make an 'ownership request' on rubygems.org and have current owners able to approve by clicking on a link or otherwise not having to go to the command line would reduce a lot of friction

RFC for said feature, talks of being able to flag gems by owners as 'looking for help' (adoption), following which other users can make 'adoption requests' (adoption_request.opened?). These requests can be either 'approved' (adoption_request.approved?) or 'rejected' (adoption_request.closed?). Differentiation between just looking for help and looking for owner/transfer can be communicated through note field of adoption. Approval of adoption request will only add requester as owner, and not remove any existing owner.
This PR allows adoption requests to be sent whether gem owner is actually looking for help (created adoption) or not. On our help site, number of tickets asking gem transfer for allegedly not-so-used gem is "too damn high". All we do on those tickets is add owner of gem to the ticket and hope they reply. Allowing adoption request being sent from site will automate some of our work. Also, I hope it will further discussion about ways of dealing with AWOL owners. Argument that an active user could have better use of a gem namespace currently occupied by an empty gem, with no releases in 5 years, under 2000 total downloads and whose owner has been unresponsive for over a year seems justifiable to me. Perhaps we can open adoption requests for upvote by other users and given gem's popularity, an upvote threshold could approve adoption. My numbers are entirely random. Coming to agreement on what constitutes an AWOL user and a not-so-used gem is no doubt going to be strenuous. I would like to think, this PR is a step in that direction.

Todo (this PR)

  • integration tests
  • mobile view
  • internationalization
  • recent adoptions and adoption request approvals page
  • badge for gems looking for adoption

Todo (future PRs)

  • pagination in adoptions view
  • rate limit on adoption applications and gem transfers per user.
  • search adoptions
  • gem page for adoption applications approvals history

More todo

These are very broad and I am not going to work on them, more discussion/agreement is needed. Probably it will also mean we finalize our terms of use (#1336). I will open an issue with my suggestions when I reach here. I need to build a more-informed opinion.

  • approval of adoption requests on basis of gem's last version release, total and monthly downloads, owner's unresponsiveness, presence of useful code in package (a lot of gems are just empty 馃毃 ).
  • allow users to approve adoption request by voting
  • ability to retire gems

馃捇 shots


I had started working on this before npm/event-stream situation. Above was my pitch which still holds, except it doesn't talk of what does ownership transfer means to users of the gem. How can we improve trust of users in code they are installing? This is not the first time this question has come up, and I would say there is fair consensus on TUF:

We plan to implement TUF on top of RubyGems in the future, and eventually deprecate the (not really secure) current gem signing system. In the meantime, treat signed gems with a large grain of salt. (source)

Problem with TUF as put by tarcieri is: "TUF is, well, tough" 馃弸 . Last time I took it up, I ran out of (my) time before I could contribute anything useful. It is still on my TODO but would it be considered blocker for this PR?
Gem adoption in a way makes ownership transfer more explicit than what is already possible with gem owner -a. Gem adoption discussion mentions of safely retiring a gem and same comes up in https://github.com/rubygems/rubygems/issues/2496.

See also: rubygems/adoption-center

Copy link
Contributor

olleolleolle left a comment

I read this change carefully, and suggested wording changes.

If you like them, that's fine. If not, that's also fine.

Thanks for adding this piece of functionality.

app/views/adoptions/_default.html.erb Outdated Show resolved Hide resolved
app/views/adoptions/_default.html.erb Outdated Show resolved Hide resolved
app/views/adoptions/_request.html.erb Outdated Show resolved Hide resolved
<%= render partial: "adoptions/show", locals: { adoption: adoption } %>
<% elsif signed_in? %>
<div class="gem_intro">
<h3 class="t-list__heading">Request adoption :</h3>

This comment has been minimized.

Copy link
@olleolleolle

olleolleolle Dec 7, 2018

Contributor
Suggested change
<h3 class="t-list__heading">Request adoption :</h3>
<h3 class="t-list__heading">Request adoption:</h3>

This comment has been minimized.

Copy link
@sonalkr132

sonalkr132 Dec 10, 2018

Author Member

are you sure there will be no space before colon?
https://english.stackexchange.com/questions/67394/spaces-around-a-colon

@bf4

This comment has been minimized.

Copy link
Contributor

bf4 commented Dec 7, 2018

@sonalkr132 @olleolleolle fwiw, here's notes I wrote up on 2016 from a discussion with @qrush that's been sitting in the Bundler slack

Kudos for taking this on!

Here's my notes with a bit of interp; I think we discussed mostly
prioritizing the ability for something to ask to be an owner and
letting an owner approve just by clicking a link in an email

RubyGem has_one OwnershipRequest
OwnershipRequest
- description   [Request Message]
- contact_email [EmailAddress]
- closed [bool] # or destroy?
On Gem page, a logged in gem owner
sees in 'admin' section (above the 'links') section
click 'request ownership'
enters a request description and a contact email
and clicks create
On Gem page, a logged in gem owner
see in 'admin' sections 
click 'edit ownership request'
can modify any details or click 'close request'
and clicks update
On Gem page, for get with open Ownership Request
any user sees a banner at the top with the body of the
ownership request and the contact email
RubyGem belongs_to OwnershipApplication
- user_id [ Integer]
- body # request message [String] max 255 char
- owner_id [Integer]
- ruby_gem_id [Integer]
- timestamps
On Gem page, a logged in User
Sees in the 'links' section
click request ownership
an OwnershipApplication is created for that user_id and with the message
owners gets email with 'authorize' link
owner clicks link
link is at /ownerships/approve/:sgid
link uses MessageVerifier sgid to verify request
ownership application is updated with the approving owner_id
requesting user is added as an owner to request gem
On Gem page with banner, a logged in user....?

the tl;dr as I see it is that for some reason, getting someone to gem owner benjamins -a davids@davidsgems.tld can stall or turn into a help ticket someone needs to deal with. This proposal turns that into please click here

@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch from 3a5b24d to 10080fb Dec 8, 2018
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch 6 times, most recently from 47e48eb to 6c31df9 Dec 8, 2018
@sonalkr132 sonalkr132 closed this Dec 10, 2018
@sonalkr132 sonalkr132 reopened this Dec 10, 2018
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch 2 times, most recently from 838e63c to fc57b7c Dec 10, 2018
@sonalkr132

This comment has been minimized.

Copy link
Member Author

sonalkr132 commented Dec 10, 2018

Thank you for your input. @olleolleolle I have updated your suggestions in i18n commit. I hope you found email bodies presentable as you didn't leave any suggestion on them.
@bf4 I have updated my PR as per your spec, except for a few cases:

OwnershipRequest and OwnershipApplication

IMHO, Adoption and AdoptionRequest better communicate transfer of gem/namespace.

contact_email [EmailAddress]
click 'edit ownership request'

Adoption belongs to user. this is not needed. I would consider edit a nice to have too.

any user sees a banner at the top with the body of the..
body # request message [String] max 255 char

I will add a small github button like tag next to gem name where adoption exists. I think we should not limit characters of body/note, as both adoption and adoption requests are better when detailed with responsibilities and acceptance terms (check screenshots).

owners gets email with 'authorize' link .. /ownerships/approve/:sgid

If we are to store approver (owner_id/approver_id), we would need to create a token per owner per adoption request(one more relation). Previously, we had issue of email token being leaked as referrer url. I get the idea of making things easier but I think it's best if we avoid email tokens. This PR, sends note of adoption request along with link to gem adoptions path (where owner can approve/close requests after login).
We can set up approval by replying to email thread if @dwradcliffe thinks its something we can/should do.

Lastly, AdoptionRequest also has a enum status field, as owners can close/reject adoption requests and we shouldn't delete the record for that.

@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch from fc57b7c to a7d21e3 Dec 11, 2018
@sonalkr132 sonalkr132 removed the discussion label Dec 11, 2018
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch from a7d21e3 to 37eb900 Dec 11, 2018
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch from 37eb900 to 9411018 Jan 4, 2019
@sonalkr132 sonalkr132 removed the needs-rebase label Jan 4, 2019
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch 5 times, most recently from 9015558 to b36f01d Feb 11, 2019
@sonalkr132

This comment has been minimized.

Copy link
Member Author

sonalkr132 commented Feb 13, 2019

I have added a sitewide page for adoptions index and a tag on gem (seeking adoption) show page.
This seems good to merge if there are no objections. There are a few left over things like pagination and email styling, which I intend to cover in follow up PRs.

@ioquatix

This comment has been minimized.

Copy link

ioquatix commented Mar 11, 2019

I really like this idea. Is it deployed yet?

I'm wondering if this forms part of a wider discussion about how to handle gems which are abandoned by their author.

Namespace is a limited resource and there are a ton of gems which were published in 2010-2012 which only ever had a handful of 0.x.y releases, < 10,000 downloads and are tying up valuable namespace which is making it hard for new contributors. Is there a good solution to this problem?

@ioquatix

This comment has been minimized.

Copy link

ioquatix commented Mar 11, 2019

Just dumping some ideas for further discussion:

How to decide if gems are "unmaintained"

Is the gem still in use?

Can we track monthly downloads? If so, we have a pretty good statistic on whether a gem is in use or not.

  • Are reverse dependencies still active?

Is the gem stable or in development?

Gems which have a "stable" release should have a higher threshold for adoption.

  • If the gem has a 1.x release, we can consider it to be stable.
  • If the gem only has 0.x releases, we can consider it to be in development.

Is the source code still available?

  • Validation on whether the source code is still available, e.g. follow the homepage link, does it give a 404 error? (tons of old/unmaintained gems = 404)

This could also be a warning sent to users - e.g. request that the URL is updated.

Is the user still active?

If users are no longer active, and the gem is no longer being maintained, this is a strong signal that someone else could step in and maintain the gem or reuse the namespace.

  • Validation on whether the user is responding to email: email all Rubygem authors once a year, perhaps with a summary of all their gems, and also ask them to confirm their account - maybe also a good opportunity to get people to update to 2FA.

Logic

Here is some pseudo code. Feel free to play around with the ideas.

class Gem
	def in_use?
		if monthly_downloads.last > 1000
			return true
		else
			reverse_dependencies.any?(&:in_use?)
		end
	end
	
	def maintained?
		author.logged_in_recently? && updated_at > 3.years.ago
	end
	
	def valid_metadata?
		response = get(homepage_url)
		
		!response.not_found?
	end
	
	def stable?
		version >= "1.0.0"
	end
	
	def adoption_threshold
		if stable? && maintained?
			return 6.years
		else
			return 3.years
		end
	end
	
	def automatic_adoption?
		last_released_at < adoption_threshold.ago
	end
end

We could also add a lock flag for gems which are critical, or part of stdlib, etc.

@ioquatix

This comment has been minimized.

Copy link

ioquatix commented Mar 12, 2019

I had one more idea about this, which would be an interesting thing I could do in isolation: upon downloading the gem and installing it (and dependencies), does the code load and do tests run?

There are a couple of ways this could work: using the declared minimum version of Ruby, or testing on all current rubies. It would give a good baseline as to whether gems were actually functioning. Given a gem that's unmaintained, not stable, and non-functional, is there an obligation by RubyGems to continue hosting it?

What could be interesting, is to add an API for tagging gems. If this was the case, I could, for example, build a local test server and actually do this, and then tag gems with the appropriate metadata.

sonalkr132 added 16 commits Nov 18, 2018
adoption aims to facilitate gem ownership transfer between users,
whether requested by gem owner or any rubygems.org user. ownership transfer
is already possibly by `gem owner -a/r` command, however to quote bf4:
> Being able to make an 'ownership request' on rubygems.org and have
current owners able to approve by clicking on a link or otherwise not
having to go to the commandline would reduce a lot of friction

RFC for said feature, talks of being able to flag gems by owners
as 'looking for help' (`adoption.seeked?`), following which other users
can make 'adoption requests' (`adoption.requested?`). These requests can be either
'approved' (`adoption.approved?`) or 'rejected' (`adoption.rejected?`).
Differentiation between just looking for help and looking for
owner/transfer can be communicated through `note` field of adoption.
Approval of adoption request with only add requester as owner, and not
remove any existing owner.
I have chosen to use a single table/model for both flagging a gem as
up for adoption and making adoption requests, (contrary to https://github.com/rubygems/adoption-center)
for both flagging a gem as up for adoption and making adoption requests,
because those two things are not different except status field, also
because I want to allow adoption requests to be sent whether gem owner
is actually looking for help. On our help site, number of tickets asking
gem transfer is "too damn high".
All we do on those tickets is add owner of gem to the ticket and hope they reply.
Allowing adoption request being sent from site will automate some of our
work. Also, I hope it will further discussion about ways of dealing with
unresponsive owners.
Only owner of gem should be able to put gem for adoption (seeked status)
Adoption update will only be of status (requested -> canceled, requested -> approved, seeked -> canceled, seeked -> approved).
Approval of adoption doesn't remove existing owners.

index action will show adoption request made by all users to gem owner.
Multiple adoption requests can be approved. Adoption requests will show up
in view unless canceled by gem owner or requester.
Past of seek is sought, not seeked. I was looking for a word equivalent
of `put up for (adoption)` and has past form ending with `ed`.

Moved section shown when gem has no open adoption to a partial.
Partial for collection view of adoption makes everything simpler.
Ensures only owner can approve adoption request
This was needed because we should save owner_id with approvals and
timestamp.
It will still have null value for adoption requests where status is
opened
but it is not as bad as null values with adoptions records created
by owners(previously).

I had started with one model to keep my change as small as possible.
Also changes adoption to singular resource (`/gems/some/adoption` makes
more sense). using `/adoption` for indexing of all adoptions.
Template names are changed to better reflect content of file and
convention.
Timestamp was added to show on adoption index page.
Adds validation for presence of note.
@sonalkr132 sonalkr132 force-pushed the sonalkr132:gem-adoption branch from b36f01d to 8c919bb Apr 8, 2019
@sonalkr132 sonalkr132 referenced this pull request Apr 28, 2019
2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can鈥檛 perform that action at this time.