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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove non-transparent requirement added to prerelease gems #7226

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Dec 7, 2023

What was the end-user or developer problem that led to this PR?

I recall this requirement confusing me while working in the resolver, since it could lead to different resolution process when working in master (2.5.0.dev currently) than when working with a released version.

It feels non transparent and too magical. I think we can safely assume these days that all RubyGems and Bundler versions that will ever bundle a new gem created in 2023 support prereleases.

So this non transparent requirement is not necessary. It should be the gem author to explicitly add this kind of constraint, not RubyGems. Setting the version of a gem should do just that, set the version.

This has also caused Circular require issues in the past, and has special conditions attached to workaround that.

What is your fix for the problem, implemented in this PR?

Remove the extra constraint.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez force-pushed the remove-hidden-constraint-on-rubygems-on-prerelease-versions branch from 3a7a5b0 to d86997e Compare December 7, 2023 14:04
Copy link
Member

@martinemde martinemde left a comment

Choose a reason for hiding this comment

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

It seems extremely unlikely that this would still be necessary. That is an old RubyGems. Just update the comment before merging.

@@ -2682,11 +2682,6 @@ def version=(version)
@version = Gem::Version.create(version)
return if @version.nil?

# skip to set required_ruby_version when pre-released rubygems.
# It caused to raise CircularDependencyError
Copy link
Member

Choose a reason for hiding this comment

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

Need to update the comment on def version= above as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that!

@martinemde
Copy link
Member

Ready to merge after the comment is fixed. (just a note to self, since I almost came back and merged this)

I think we can safely assume these days that all RubyGems and Bundler
versions that will ever bundle a new gem created in 2023 support
prereleases.

So this non transparent requirement is not necessary.

In my opinion, it should be the gem author to explicitly add this
constraint, not RubyGems.
@deivid-rodriguez deivid-rodriguez force-pushed the remove-hidden-constraint-on-rubygems-on-prerelease-versions branch from d86997e to b165e6d Compare December 11, 2023 17:18
@deivid-rodriguez deivid-rodriguez merged commit 21b4f45 into master Dec 11, 2023
60 checks passed
@deivid-rodriguez deivid-rodriguez deleted the remove-hidden-constraint-on-rubygems-on-prerelease-versions branch December 11, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants