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

[github] Remove Ruby 2.5+2,6 tests, add Ruby 3.0+3.1 tests, update dev gems #68

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

dafyddcrosby
Copy link
Contributor

Several smaller commits in one PR (check the commits tab for blow-by-blow), setting the stage for getting this stuff on modern versions

  • Ignore local copy of rubocop config
  • Bump actions/checkout to get rid of CI errors
  • Bump rubocop to same version as facebook/chef-cookbooks
  • Remove chef-dk (it's super dead now)
  • Explicitly use chef-zero >=15, since it's needed for Ruby 3

gem 'simplecov'
gem 'webrick' # chef-zero dep
Copy link
Contributor

Choose a reason for hiding this comment

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

we shouldn't explicitly set dependencies of our dependencies, chef-zero should have this in it's deps. We should only list our direct deps.

@@ -4,12 +4,13 @@ gemspec

group :development, :test do
gem 'between_meals', :git => 'https://github.com/facebook/between-meals'
gem 'chef-dk', '~> 3.13.0'
gem 'chef-zero'
gem 'chef', '= 16.18.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be chef-workstation these days.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like to get away from chef-workstation as a dependency, since it's not entirely clear what's actually needed from chef-workstation. The chef-dk->chef-workstation transition was kind of awful (that's a beer conversation), and I'd like to avoid that again ;-).

That said, knife was moved workstation in Chef 17, so that should be explicitly specified, too. Will try to have those fixes up in the next few days

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, so this is tricky.

TECHNICALLY GD, BM, and TT don't have a direct dependency on dk/ws.

However, they're generally intended to be installed as a devtools along side dk/ws. And we try to move more and more stuff into WS so we don't have to manage it in a fb-chefdk-gems sorta way. That's true, for example, with markdownlint, we got that into WS so we could stop managing it ourselves.

You're right though that strictly speaking, we don't need ws/dk, we just use those to provide our actual deps.

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

Successfully merging this pull request may close these issues.

3 participants