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

Be more selective about what dev gems we bundle #1393

Merged
merged 1 commit into from
Sep 21, 2017
Merged

Be more selective about what dev gems we bundle #1393

merged 1 commit into from
Sep 21, 2017

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Sep 14, 2017

berkshelf no longer has a guard group in the gemfile so remove that exclusion
chef no longer has a changelog group so remove that exclusion
foodcritic has no changelog group, but it does have a development group: ronn, pry
test-kitchen (master currently) has a debug group and a docs group: yard, pry, pry-byebug, and pry-stack_explorer
inspec no longer has a changelog group, but it does have deploy, tools, and maintenance groups: pry, rb-readline, license_finder, tomlrb, octokit, netrc, and inquirer

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

`berkshelf` no longer has a guard group in the gemfile so remove that exclusion
`chef` no longer has a changelog group so remove that exclusion
`foodcritic` has no changelog group, but it does have a development group: ronn, pry
`test-kitchen` (master currently) has a debug group and a docs group: yard, pry, pry-byebug, and pry-stack_explorer
`inspec` no longer has a changelog group, but it does have deploy, tools, and maintenance groups: pry, rb-readline, license_finder, tomlrb, octokit, netrc, and inquirer

Signed-off-by: Tim Smith <tsmith@chef.io>
@tas50 tas50 requested a review from a team September 14, 2017 21:15
Copy link

@jjasghar jjasghar left a comment

Choose a reason for hiding this comment

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

Great start. Stripping down things we don't need to ship is always a good idea.

@coderanger
Copy link
Contributor

The inspec one we might regret since this will yank pry from that appbundle, not sure how many normal people use pry for debugging inspec tests though. Otherwise 👍

@adamleff
Copy link
Contributor

The InSpec Shell it built on pry, but pry is already listed as a dependency in the gemspec so we should be OK.

https://github.com/chef/inspec/blob/master/inspec.gemspec#L37

@lamont-granquist
Copy link
Contributor

pretty certain that what we ship is just what is in the Gemfile.lock and that these get applied to the merged Gemfile.lock which use in order to test the gems, and that by removing the development gems from some of these that you'll have broken the chefdk test builds. and by removing gem groups here you don't affect what we ship, only what we install on the testers.

@lamont-granquist
Copy link
Contributor

so please test this manually with an ad-hoc build in manhattan before shipping

@lamont-granquist
Copy link
Contributor

tested with an adhoc in manhattan and it got through the test phase all green (still pretty sure this does not affect shipping gems at all)

@tas50 tas50 merged commit 7827fb8 into master Sep 21, 2017
@tas50
Copy link
Contributor Author

tas50 commented Sep 21, 2017

Even if all we do is limit test deps that's fine by me.

@tas50 tas50 deleted the appbundle branch September 21, 2017 23:35
@chef-boneyard chef-boneyard locked and limited conversation to collaborators Feb 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants