-
Notifications
You must be signed in to change notification settings - Fork 2
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
Rubocop and comment out unneeded gems #29
Conversation
8958c87
to
beb3779
Compare
@@ -15,16 +15,16 @@ gem 'sqlite3' | |||
# Use Puma as the app server | |||
gem 'puma', '~> 3.7' | |||
# Use SCSS for stylesheets | |||
gem 'sass-rails', '~> 5.0' | |||
# gem 'sass-rails', '~> 5.0' |
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.
Why not just delete instead of comment out?
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.
Excellent question! All of this stuff was added by the rails generator, including pinning to versions. I was feeling tentative, so I commented stuff out. I would be happy to remove cruft if we prefer -- we can always get it back from git history or another rails app if we need it. What do you think?
See also #30 which is about removing directories and the like for unused stuff -- are we comfy with removing all this stuff?
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.
My opinion on this one is that we might as well remove this stuff from the Gemfile. These are fairly small changes that would be easy to reinstate, even just by - as you say - running the rails generator again to figure out what we're missing.
I'm less certain about #30 - I believe that removing some of those directories can come back to bite us later and it might be worth just leaving them in there?
Resolves #22
Also commented out gems related to html or js or css or windows, as this is going to be json or xml responses only. Happy to move that commit to a separate PR.
We can twiddle our rubocop settings if these initial ones prove annoying ... or nominate your favorites in the comments!