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

Update dependency versions inside gemspec #413

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions fog-azure-rm.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'minitest', '~> 5.8.4'
spec.add_development_dependency 'simplecov'
spec.add_development_dependency 'codeclimate-test-reporter' , '~> 1.0.0'
spec.add_dependency 'fog-core', '~> 1.43'
spec.add_dependency 'fog-json', '~> 1.0.2'
spec.add_dependency 'fog-core'
spec.add_dependency 'fog-json'
Copy link
Author

Choose a reason for hiding this comment

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

@maham-nazir-confiz I have removed the version dependency here. Could you please take a look?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll get back to you after reviewing the changes.

Regards

Copy link
Author

Choose a reason for hiding this comment

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

@maham-nazir-confiz Any updates on this?

spec.add_dependency 'azure_mgmt_compute', '~> 0.9.0'
spec.add_dependency 'azure_mgmt_resources', '~> 0.9.0'
spec.add_dependency 'azure_mgmt_storage', '~> 0.9.0'
Expand All @@ -32,7 +32,7 @@ Gem::Specification.new do |spec|
spec.add_dependency 'azure_mgmt_traffic_manager', '~> 0.9.0'
spec.add_dependency 'azure_mgmt_sql', '~> 0.9.0'
spec.add_dependency 'azure_mgmt_key_vault', '~> 0.9.0'
spec.add_dependency 'azure-storage', '= 0.11.5.preview'
spec.add_dependency 'azure-storage', '~> 0.11.preview'
Copy link

Choose a reason for hiding this comment

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

Can we use the latest version 0.15.0.preview here?

Choose a reason for hiding this comment

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

@bguban, ~> 0.11.preview will take 0.15.0.preview too.

Choose a reason for hiding this comment

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

~> 0.11.preview means >= 0.11 including preview and < 1.0

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain how it will take 0.15.0.preview as well? When I install the gem with these dependencies, it just shows as 0.11.preivew

Choose a reason for hiding this comment

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

According to thoughtbot (specifically the second part - ~> 1.1 example) this will result with >= 0.11 and < 1.0 as I have mentioned before. According to rubygems pattern guide

Using ~> with prerelease versions will restrict you to prerelease versions only.

Which should be good, since Azure only releases .preview versions to rubygems

We already tested it, and 0.15.preview was pulled.

Copy link

Choose a reason for hiding this comment

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

@maham-nazir-confiz, @ShimShtein maybe as possible solution is just to increment the major version of the fog-azure-rm and use the latest versions of dependent gems in it. If somebody needs backward compatibility they would use the current version, who don't need it could use the newer version of the gem.

I agree, upgrading to later gem versions would resolve much of these issues.

Copy link
Author

Choose a reason for hiding this comment

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

@bguban This can be a possible solution.
@maham-nazir-confiz your thoughts on @bguban 's comment?

Choose a reason for hiding this comment

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

@maham-nazir-confiz, Actually it depends on the user to limit nokogiri version. I mean the user can limit nokogiri version in his own Gemspec file like this:

gem 'fog-azure-rm' # require our library
gem 'nokogiri', '<=1.9.1' # if the user wants ruby 2.0 compatibility

azure-storage does not require newer 'nokogiri', it sets only lower limit requirement. It's up to user to set up higher limit, if the user needs it.

Other option would be adding an if condition into fog-azure-rm.gemspec, but I am not sure it will be parsed well by rubygems.

Copy link
Author

Choose a reason for hiding this comment

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

@ShimShtein That's right, users can set the upper limit of nokogiri, if required, in their own gemspec.
@maham-nazir-confiz What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

@maham-nazir-confiz Could you please let us know if the approach suggested against nokogiri failure works for you?

spec.add_dependency 'vhd', '0.0.4'
spec.add_dependency 'mime-types', '~> 1.25'
spec.add_dependency 'mime-types', '> 1.25', '< 4.0'
end