-
Notifications
You must be signed in to change notification settings - Fork 375
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
RFC:Remove devtools from main gem set #2421
base: master
Are you sure you want to change the base?
Conversation
Gemfile
Outdated
group :appraisal do | ||
gem 'appraisal', '~> 2.2' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach 1
Very specific groups, pretty much matching the tool name: e.g. appraisal
, rubocop
, yard
, sorbet
, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be extremely granural, not too keen on that solution.
Gemfile
Outdated
group :release do | ||
gem 'pimpmychangelog', '>= 0.1.2' # Formats the CHANGELOG.md file | ||
gem 'redcarpet', '~> 3.4' if RUBY_PLATFORM != 'java' # Used by YARD to generate docs | ||
gem 'yard', '~> 0.9' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach 2
Locally grouped gems, matching the high-level activity they are part of: e.g. release
, static_check
, benchmark
, test
, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach 2a
Locally grouped gems, but more granular then 2
: e.g. changelog
(has pimpmychangelog
), docs
(has redcarpet
& yard
), lint
(has rubocop*
), type_check
(has sorbet
& spoom
) etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this better than Approach 1.
I like Approach 2 better than Approach 2a as I don't think we need to go that deep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have strong thoughts either way, but I think coarser-grained groups seem like a good enough start, and we could always break them up as needed later.
Gemfile
Outdated
gem 'yard', '~> 0.9' | ||
end | ||
|
||
group :static_check do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually call that just :check
or :lint
(not the least because the sorbet
gem is not static: it evaluates files!)
I'm very much liking this, if only because it should resolve the |
It would be best to have the test matrix run with only gems relevant to the test at hand.
We have many development tools in our Gemfile, which get included in all gem set combinations, in some occasions causing conflicts.
It seems more practical to me, at this point, to separate everything that we know is not necessary to run
ddtrace
or its test suite into separate "groups", and keep what's left as "essential to ddtrace or testing ddtrace".This PR, at this point, is just a sketch for possible approaches we can take.