Reduce production gem size #345
Conversation
tagliala
force-pushed
the
chore/reduce-production-gem-size
branch
from
2e0b5ae
to
fe42265
Compare
memo_wise.gemspec
Outdated
| end | ||
| spec.files = Dir.glob( | ||
| '{CHANGELOG.md,LICENSE.txt,README.md,lib/**/*.rb}', | ||
| File::FNM_DOTMATCH |
There was a problem hiding this comment.
Why do we need FNM_DOTMATCH? Wouldn't it be the same without it since none of these files begin with .?
There was a problem hiding this comment.
Thanks, it is not needed, it is also wrong for this use case. I've used it somewhere else to exclude directories (., ..) but this is not the case
tagliala
force-pushed
the
chore/reduce-production-gem-size
branch
3 times, most recently
from
018b18b
to
d4fece2
Compare
|
Main / CI (3.3) action should be probably set to "required" in repo's ruleset settings
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 190 190
Branches 90 90
=========================================
Hits 190 190 ☔ View full report in Codecov by Sentry. |
memo_wise.gemspec
Outdated
| f.match(%r{^(test|spec|features)/}) | ||
| end | ||
| end | ||
| spec.files = Dir.glob('{CHANGELOG.md,LICENSE.txt,README.md,lib/**/*.rb}') |
There was a problem hiding this comment.
Looks like RuboCop wants these 's to be "s. Other than that and my other comment I think this is good!
CHANGELOG.md
Outdated
| @@ -9,6 +9,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm | |||
|
|
|||
| **Gem enhancements:** | |||
|
|
|||
| - Reduced production gem size by 9 times [[#345](https://github.com/panorama-ed/memo_wise/pull/345)] | |||
There was a problem hiding this comment.
To be honest I always find statements like this confusing (would reduced size by 1x mean no reduction or a 100% reduction?). Would you be open to changing this to absolutes, e.g.:
Reduced unzipped gem size from 173KB to 19KB
or something similar?
There was a problem hiding this comment.
Much better, thanks
tagliala
force-pushed
the
chore/reduce-production-gem-size
branch
from
d4fece2
to
fe2deb1
Compare
Include only files that are needed for the production gem. Also avoids to use git to allow to build the package in an environment that does not have git (on purpose). ### Before 173K ``` .dokaz .github/PULL_REQUEST_TEMPLATE.md .github/dependabot.yml .github/workflows/dependency-review.yml .github/workflows/main.yml .gitignore .rspec .rubocop.yml .ruby-version .yardopts CHANGELOG.md CODE_OF_CONDUCT.md Gemfile Gemfile.lock LICENSE.txt README.md Rakefile benchmarks/Gemfile benchmarks/benchmarks.rb bin/console bin/setup lib/memo_wise.rb lib/memo_wise/internal_api.rb lib/memo_wise/version.rb logo/logo.png memo_wise.gemspec ``` ### After 19K ``` CHANGELOG.md LICENSE.txt README.md lib/memo_wise.rb lib/memo_wise/internal_api.rb lib/memo_wise/version.rb ``` Ref: https://docs.rubocop.org/rubocop-packaging/cops_packaging.html#packaginggemspecgit
tagliala
force-pushed
the
chore/reduce-production-gem-size
branch
from
fe2deb1
to
17beca6
Compare
|
@tagliala I actually have a question for you about this: #346 (comment) EDIT: Whoops, never mind I'd missed your response! |
|
@tagliala I forgot to mention this yesterday, but this improvement has been published to RubyGems as v1.10.0. Thanks! |
|
I've seen that, downloaded, and tested, thanks! |
Hello,
please check if this makes sense and if the production gem should also include other files
Include only files that are needed for the production gem.
Also avoids to use git to allow to build the package in an environment
that does not have git (on purpose).
Before
173K
After
19K
Before merging:
Copy the table printed at the end of the latest benchmark results into theREADME.mdand update this PRCHANGELOG.md, add an entry following Keep a Changelog guidelines with semantic versioning