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

Avoid shipping development cruft files in gem releases #1585

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Jul 9, 2021

The spec.files setting controls which files get shipped when the gem gets packaged/installed.

We still use the default setup of everything in git, other than spec/test/features folders. The problem is, that this means that
we're shipping the integration/, gemfiles/, .circleci/ and a bunch of extra development stuff.

(Full list at: https://gist.github.com/ivoanjo/0e3aa44ae9e3ca7a14194d8318f52b62)

I've pared this list down to include only things useful. After this change, we include the following files in a release (with current master):

.gitignore
.yardopts
CHANGELOG.md
CONTRIBUTING.md
LICENSE
LICENSE-3rdparty.csv
LICENSE.Apache
LICENSE.BSD3
NOTICE
README.md
bin/ddtracerb
ddtrace.gemspec
docs/DevelopmentGuide.md
docs/GettingStarted.md
docs/ProfilingDevelopment.md
lib/*

This means that gem releases will be smaller and have less irrelevant things.

The `spec.files` setting controls which files get shipped when the
gem gets packaged/installed.

We still use the default setup of everything in git, other than
spec/test/features folders. The problem is, that this means that
we're shipping the `integration/`, `gemfiles/`, `.circleci/` and
a bunch of extra development stuff.

(Full list at:
<https://gist.github.com/ivoanjo/0e3aa44ae9e3ca7a14194d8318f52b62>)

I've pared this list down to include only things useful. After this
change, we include the following files in a release (with current
master):

```
.gitignore
.yardopts
CHANGELOG.md
CONTRIBUTING.md
LICENSE
LICENSE-3rdparty.csv
LICENSE.Apache
LICENSE.BSD3
NOTICE
README.md
bin/ddtracerb
ddtrace.gemspec
docs/DevelopmentGuide.md
docs/GettingStarted.md
docs/ProfilingDevelopment.md
lib/*
```

This means that gem releases will be smaller and have less irrelevant
things.
@ivoanjo ivoanjo requested a review from a team July 9, 2021 12:12
@codecov-commenter
Copy link

Codecov Report

Merging #1585 (6976772) into master (01eea6a) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1585      +/-   ##
==========================================
+ Coverage   98.22%   98.25%   +0.03%     
==========================================
  Files         862      894      +32     
  Lines       41704    42879    +1175     
==========================================
+ Hits        40962    42131    +1169     
- Misses        742      748       +6     
Impacted Files Coverage Δ
lib/ddtrace/contrib/rails/log_injection.rb 76.92% <0.00%> (-7.46%) ⬇️
lib/ddtrace/ext/environment.rb 93.33% <0.00%> (-6.67%) ⬇️
lib/ddtrace/profiling/scheduler.rb 90.90% <0.00%> (-6.66%) ⬇️
lib/ddtrace/workers/polling.rb 96.42% <0.00%> (-3.58%) ⬇️
lib/ddtrace/configuration/components.rb 98.19% <0.00%> (-1.81%) ⬇️
spec/ddtrace_integration_spec.rb 95.06% <0.00%> (-1.24%) ⬇️
lib/ddtrace/metrics.rb 95.62% <0.00%> (-1.02%) ⬇️
spec/support/synchronization_helpers.rb 83.33% <0.00%> (-0.54%) ⬇️
spec/ddtrace/metrics_spec.rb 99.61% <0.00%> (-0.39%) ⬇️
lib/ddtrace/contrib/qless/patcher.rb 94.11% <0.00%> (-0.33%) ⬇️
... and 171 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edb8b89...6976772. Read the comment docs.

Comment on lines +34 to +37
.reject { |f| f.match(%r{^(test|spec|features|[.]circleci|[.]github|[.]dd-ci|benchmarks|gemfiles|integration|tasks)/}) }
.reject do |f|
['.dockerignore', '.env', '.gitattributes', '.gitlab-ci.yml', '.rspec', '.rubocop.yml',
'.rubocop_todo.yml', '.simplecov', 'Appraisals', 'Gemfile', 'Rakefile', 'docker-compose.yml'].include?(f)
Copy link
Member

Choose a reason for hiding this comment

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

What this point, what do you think of listing the included files instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not the biggest fan of that, since I've seen quite a few broken releases on rubygems.org due to "it worked on my machine but we forgot to update the files list and so the packaged gem is missing stuff".

On the other hand, those projects usually specified all files by name, and I think you may be suggesting to just specify the ones on the the root, which is a smaller subset, but... I'd still go with the safer default of excluding what we don't want.

@ivoanjo ivoanjo merged commit de3b674 into master Jul 13, 2021
@ivoanjo ivoanjo deleted the ivoanjo/avoid-shipping-dev-cruft-in-gem-releases branch July 13, 2021 08:03
@github-actions github-actions bot added this to the 0.52.0 milestone Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants