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

Add a minimum ruby version to the generated gemspec #7209

Merged
3 commits merged into from
Jun 24, 2019
Merged

Add a minimum ruby version to the generated gemspec #7209

3 commits merged into from
Jun 24, 2019

Conversation

deivid-rodriguez
Copy link
Member

This PR is a finished version of #4397.

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

The problem was that the generated gemspec for new gems does not impose a minimum ruby version.

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

My fix is to set the minimum ruby version in the generated gemspec to the minimum ruby version supported by bundler itself.

Why did you choose this fix out of the possible options?

I chose this fix because it's simple.

@deivid-rodriguez
Copy link
Member Author

@bundlerbot r=hsbt

ghost pushed a commit that referenced this pull request Jun 24, 2019
7209: Add a minimum ruby version to the generated gemspec  r=hsbt a=deivid-rodriguez

This PR is a finished version of #4397.

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

The problem was that the generated gemspec for new gems does not impose a minimum ruby version.

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

My fix is to set the minimum ruby version in the generated gemspec to the minimum ruby version supported by bundler itself.

### Why did you choose this fix out of the possible options?

I chose this fix because it's simple.

Co-authored-by: David Rodríguez <[email protected]>
Co-authored-by: Miklos Fazekas <[email protected]>
@@ -12,6 +12,7 @@ Gem::Specification.new do |spec|
<%- if config[:mit] -%>
spec.license = "MIT"
<%- end -%>
spec.required_ruby_version = Gem::Requirement.new(">= 2.3.0")
Copy link
Member

Choose a reason for hiding this comment

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

If I understand it will, this should be the same as https://github.com/bundler/bundler/blob/c5dcdfbc3617d1ecf551f047456d4ea03a18f930/bundler.gemspec#L34. But to prevent loading bundler gemspec during generating new gem, this is hardcoded in here, but covered by spec ensuring those two values are the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, although I didn't really consider your alternative solution: load bundler's gemspec, and load the minimum version from there. It sounds like it might be a better approach since it's more DRY and we don't need a spec to keep the minimum versions in sync... 🤔.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, I was thinking loading bundler's gemspec is overkill during generating new gem. Anyway I can inspect that option if needed. Feel free to ping me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't have a strong opinion, this is probably good enough.

@ghost
Copy link

ghost commented Jun 24, 2019

Build succeeded

This pull request was closed.
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.

4 participants