Skip to content

Add ruby 2.7 to travis#238

Closed
joshRpowell wants to merge 3 commits intorubysec:masterfrom
joshRpowell:add/ruby-27
Closed

Add ruby 2.7 to travis#238
joshRpowell wants to merge 3 commits intorubysec:masterfrom
joshRpowell:add/ruby-27

Conversation

@joshRpowell
Copy link

@joshRpowell joshRpowell commented Dec 26, 2019

@joshRpowell
Copy link
Author

joshRpowell commented Jan 21, 2020

@postmodern looks like a combination of #236 #237 #242 gets things going again.

Lets get those merged first and I'll rebase and squash.

@postmodern
Copy link
Member

Other three PRs have been merged. Shall I "squash and merge` or do you want to rebase+squash?

homepage: https://github.com/rubysec/bundler-audit#readme

required_ruby_version: ">= 1.9.3"
required_ruby_version: ">= 2.4.0"
Copy link
Member

@postmodern postmodern Jan 29, 2020

Choose a reason for hiding this comment

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

@joshRpowell Just curious why we're dropping support for Ruby 2.3? Even Thor 1.x supports Ruby >= 2.0.0.

Copy link
Author

Choose a reason for hiding this comment

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

@postmodern following the lead in popular OSS elsewhere now that 2.3 is EOL: https://github.com/search?q=drop+ruby+2.3&type=Commits

if we decide to keep, we'll have to look through locking in dependency versions per ruby version: https://travis-ci.org/rubysec/bundler-audit/jobs/640082516?utm_medium=notification&utm_source=github_status

look forward to your thoughts. will polish off whats need.

Copy link
Member

Choose a reason for hiding this comment

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

Since the whole purpose of the bundles is to have Gemfile.locks to test the Scanner class against, we should probably just commit the Gemfile.lock files to lock down our testing dependencies. This would prevent newer versions of gems from leaking. I have some old Gemfile.lock files locally that I could commit?

Copy link
Member

Choose a reason for hiding this comment

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

Committed the Gemfile.lock files and Travis is all green. I'll probably bump the required ruby version up to 2.0.0, due to the differences between 1.9 and 2.0.

Copy link
Author

Choose a reason for hiding this comment

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

given this comment when 2.1 & 2.2 were dropped. Should 2.3 be the required ruby version?

Copy link
Member

Choose a reason for hiding this comment

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

I'm kind of opposed to using required_ruby_version as "supported ruby version". To me, "required version" means the absolute minimum version that the code needs to run. We should set required_ruby_version based on what Ruby features we're using, or if we hit some show-stopping bug in a particular Ruby version and we must blacklist the version(s) in order to prevent users from hitting the bug and constantly re-reporting the issue to use. Not to mention it would be a lot of work if every gem maintainer had to bump required_ruby_version each time another Ruby version reached EoL.

@joshRpowell
Copy link
Author

I'm kind of opposed to using required_ruby_version as "supported ruby version". To me, "required version" means the absolute minimum version that the code needs to run. We should set required_ruby_version based on what Ruby features we're using, or if we hit some show-stopping bug in a particular Ruby version and we must blacklist the version(s) in order to prevent users from hitting the bug and constantly re-reporting the issue to use. Not to mention it would be a lot of work if every gem maintainer had to bump required_ruby_version each time another Ruby version reached EoL.

Healthy discussion. 👍 Will close this and let you take you take it from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants