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

Only ship the necessary libs in the gem artifact #148

Merged
merged 3 commits into from
Dec 18, 2018
Merged

Only ship the necessary libs in the gem artifact #148

merged 3 commits into from
Dec 18, 2018

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 18, 2018

The gem artifact is not the ideal place to ship examples, contributing
docs, a rake file and various hidden files. This will greatly reduce the
size of the installed gem artifact and speed up installs on systems with
malware scanners. This drops the compressed gem size from 221k to 47k.

Signed-off-by: Tim Smith tsmith@chef.io

The gem artifact is not the ideal place to ship examples, contributing
docs, a rake file and various hidden files. This will greatly reduce the
size of the installed gem artifact and speed up installs on systems with
malware scanners. This drops the compressed gem size from 221k to 47k.

Signed-off-by: Tim Smith <tsmith@chef.io>
@jrgarcia jrgarcia self-requested a review December 18, 2018 15:16
@jrgarcia jrgarcia self-assigned this Dec 18, 2018
@jrgarcia
Copy link
Contributor

Agreed. I'll take a look at this today. Thanks for your contribution!

Copy link
Contributor

@jrgarcia jrgarcia left a comment

Choose a reason for hiding this comment

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

Once the changes requested are added, I think this looks great. Thanks for your contribution! It's much appreciated! 🎉

rbvmomi.gemspec Outdated
@@ -16,7 +16,7 @@ Gem::Specification.new do |spec|
spec.license = 'MIT'

spec.bindir = 'exe'
spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(/^test\//) }
spec.files = %w{LICENSE README.md} + Dir.glob("{lib,exec}/**/*")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you meant to put exe rather than exec in there, but that isn't really needed as you'll never require it and it's pulled into the gem through the spec.executables line below. What is needed, however, is the vmodl.db file in the root of the repository. Other than that, I think this looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Nice catch. I pushed a few for the right path.

Signed-off-by: Tim Smith <tsmith@chef.io>
rbvmomi.gemspec Outdated
@@ -16,7 +16,7 @@ Gem::Specification.new do |spec|
spec.license = 'MIT'

spec.bindir = 'exe'
spec.files = `git ls-files -z`.split("\x0").reject { |f| f.match(/^test\//) }
spec.files = %w{LICENSE README.md} + Dir.glob("{lib,exe}/**/*")
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, for another request, but could you add the vmodl.db file in there as well. It's needed for rbvmomi to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

Signed-off-by: Tim Smith <tsmith@chef.io>
Copy link
Contributor

@jrgarcia jrgarcia left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 Thanks again!

@jrgarcia jrgarcia merged commit 7e813a1 into vmware-archive:master Dec 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants