Refactor of gem files and rake tasks#157
Refactor of gem files and rake tasks#157javierav wants to merge 1 commit intoipaddress-gem:masterfrom
Conversation
ab4d5ac to
64cb4ca
Compare
sandstrom
left a comment
There was a problem hiding this comment.
Great work! 💯
I wonder if we can rename .document to .rdoc_options?
https://ruby.github.io/rdoc/RDoc/Options.html#class-RDoc::Options-label-Saved+Options
Rakefile
Outdated
| rescue LoadError | ||
| task :rcov do | ||
| abort "RCov is not available. In order to run rcov, you must: sudo gem install spicycode-rcov" | ||
| desc "Look for TODO and FIXME tags in the code" |
There was a problem hiding this comment.
I'd probably drop this, easy with project-wide search.
But I'm not against keeping it if you prefer.
Gemfile
Outdated
| end | ||
| gemspec | ||
|
|
||
| gem "minitest" |
There was a problem hiding this comment.
Any reason for dropping the group :development do?
I'd prefer keeping it, but maybe there is a good reason for dropping?
There was a problem hiding this comment.
In a gem, the Gemfile file is always used for development, so it is not necessary to differentiate by groups and they can go to the Bundler default group (:default) and thus the Gemfile file is much simpler.
Gemfile
Outdated
| end | ||
| gemspec | ||
|
|
||
| gem "minitest" |
There was a problem hiding this comment.
I think we can specify a minimum version for these gems, since they are dev-dependencies and not gem dependencies.
gem 'minitest', '~> 5.8' (or similar)
There was a problem hiding this comment.
The Gemfile.lock file is now versioned within git, so all developers will install, when they run the bundler install command, the same version of the libraries that the project requires for its development.
https://bundler.io/v2.5/man/bundle-install.1.html#THE-GEMFILE-LOCK
On the other hand, the dependencies of the gem when it is installed in other projects are marked by what is indicated in the gemspec, although this gem does not have any runtime dependencies.
There was a problem hiding this comment.
Thanks, yes I know how all this works.
However, I think it's useful that the Gemfile signals intent. For example `gem 'minitest', '~> 5.8' signals that we're not expecting minor updates to cause any trouble, so those are alright to update automatically, or without much thought, but major versions need to be more explicit.
https://stackoverflow.com/questions/9265213/should-i-specify-exact-versions-in-my-gemfile
https://www.reddit.com/r/rails/comments/sujdqk/best_practice_should_i_specify_versions_in_gem/
Can you put these back?
gem 'minitest', '~> 5.8'
gem 'rake', '~> 13.1'
gem 'simplecov', '~> 0.22', require: falseOther than this, I think this is ready to go! 💯
If I'm honest, I don't know what this file is for and who uses it. |
bfe69b5 to
a64ce60
Compare
|
I also refactored |
Gemfile
Outdated
| end | ||
| gemspec | ||
|
|
||
| gem "minitest" |
There was a problem hiding this comment.
Thanks, yes I know how all this works.
However, I think it's useful that the Gemfile signals intent. For example `gem 'minitest', '~> 5.8' signals that we're not expecting minor updates to cause any trouble, so those are alright to update automatically, or without much thought, but major versions need to be more explicit.
https://stackoverflow.com/questions/9265213/should-i-specify-exact-versions-in-my-gemfile
https://www.reddit.com/r/rails/comments/sujdqk/best_practice_should_i_specify_versions_in_gem/
Can you put these back?
gem 'minitest', '~> 5.8'
gem 'rake', '~> 13.1'
gem 'simplecov', '~> 0.22', require: falseOther than this, I think this is ready to go! 💯
This PR updates the gem development environment, some rake tasks, and the gemspec. Needs to be rebased when my other PRs are merged!