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

Cleanup dependencies #10171

Merged
merged 7 commits into from Feb 5, 2019
Merged

Cleanup dependencies #10171

merged 7 commits into from Feb 5, 2019

Conversation

@jsvd
Copy link
Member

jsvd commented Nov 21, 2018

No description provided.

Gemfile.template Show resolved Hide resolved
Gemfile.template Outdated Show resolved Hide resolved
@jsvd jsvd force-pushed the jsvd:cleanup_dependencies branch from 17237ad to afe03fe Nov 21, 2018
Gemfile.template Show resolved Hide resolved
Gemfile.template Outdated Show resolved Hide resolved
@jsvd jsvd force-pushed the jsvd:cleanup_dependencies branch from 7b7eeb2 to 5073328 Nov 22, 2018
Copy link
Member

yaauie left a comment

This PR changes quite a few of our dependencies to be more open-ended than I imagine you are intending; if we have other safeguards in place to make sure we don't accidentally consume major releases from these dependencies, my arguments may be moot.

Since the squiggle-arrow operator allows the most-precisely specified version field to be greater than or equal to what is specified, using it with a single version field (the major) presumably allows breaking changes when dependencies release new majors; this could produce hard-to-debug problems later on if/when new major versions are released.

EDIT: resolved.

logstash-core/logstash-core.gemspec Outdated Show resolved Hide resolved
logstash-core/logstash-core.gemspec Show resolved Hide resolved
logstash-core/logstash-core.gemspec Show resolved Hide resolved
@jsvd

This comment has been minimized.

Copy link
Member Author

jsvd commented Nov 27, 2018

Since the squiggle-arrow operator allows the most-precisely specified version field to be greater than or equal to what is specified, using it with a single version field (the major) presumably allows breaking changes when dependencies release new majors; this could produce hard-to-debug problems later on if/when new major versions are released.

The squiggle operator is a bit weird for majors, it behaves as "only allow this major". This can be demonstrated with:

jruby-9.2.3.0 :007 > Gem::Requirement.new("~> 1").satisfied_by?(Gem::Version.new("2.0.0"))
 => false 
jruby-9.2.3.0 :008 > Gem::Requirement.new("~> 1").satisfied_by?(Gem::Version.new("2"))
 => false 
jruby-9.2.3.0 :009 > Gem::Requirement.new("~> 1").satisfied_by?(Gem::Version.new("1.4"))
 => true 
jruby-9.2.3.0 :010 > Gem::Requirement.new("~> 1").satisfied_by?(Gem::Version.new("0.4"))
 => false 
jruby-9.2.3.0 :011 > Gem::Requirement.new("~> 1").satisfied_by?(Gem::Version.new("1.0"))
 => true
@jsvd jsvd force-pushed the jsvd:cleanup_dependencies branch from 5073328 to 5e557de Jan 30, 2019
@jsvd

This comment has been minimized.

Copy link
Member Author

jsvd commented Jan 31, 2019

jenkins test this please

@jsvd jsvd changed the title [wip] Cleanup dependencies Cleanup dependencies Jan 31, 2019
@jsvd jsvd removed the work in progress label Jan 31, 2019
@jsvd

This comment has been minimized.

Copy link
Member Author

jsvd commented Jan 31, 2019

@yaauie ci is green now, can you take another pass?

jsvd added 4 commits Nov 21, 2018
@jsvd jsvd force-pushed the jsvd:cleanup_dependencies branch from ce586e3 to 53e0977 Feb 5, 2019
@jsvd

This comment has been minimized.

Copy link
Member Author

jsvd commented Feb 5, 2019

jenkins test this please

jsvd added 2 commits Feb 5, 2019
@yaauie
yaauie approved these changes Feb 5, 2019
Copy link
Member

yaauie left a comment

LGTM 👍

logstash-core/logstash-core.gemspec Show resolved Hide resolved
logstash-core/logstash-core.gemspec Show resolved Hide resolved
@jsvd jsvd merged commit 8d19e6c into elastic:master Feb 5, 2019
1 of 2 checks passed
1 of 2 checks passed
logstash-ci Build finished.
Details
CLA Commit author has signed the CLA
Details
@jsvd jsvd deleted the jsvd:cleanup_dependencies branch Feb 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.