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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uniform paths joining in the code generated by rails new #40215

Merged
merged 1 commit into from
Dec 29, 2020

Conversation

vlado
Copy link
Contributor

@vlado vlado commented Sep 11, 2020

Summary

Adds uniformity when joining paths (Rails.root.join(...)) in the code generated by rails new command.

rails new currently generates the code with slashes in this places:

https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/test_unit/generator/templates/generator_test.rb.tt#L7
https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/storage.yml.tt#L3
https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/storage.yml.tt#L7
https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/storage.yml.tt#L21

And using arguments here:

https://github.com/rails/rails/blob/6-0-stable/railties/lib/rails/generators/rails/app/templates/config/environments/development.rb.tt#L17 (this PR changes it to slashes also).

Other Information

Based on my personal experience, having Rubocop running while teaching school kids Ruby/Rails is a huge time saver. No need to point them to same mistakes all the time. We also create new Rails apps a lot and each time config/environments/development.rb:17:6: C: Rails/FilePath: Please use Rails.root.join('path/to') instead. offence is reported. We have rule disabled for a long time and I finally gather up the courage to submit this PR 馃挭馃槃.

I understand that this change could be considered cosmetic in nature and not accepted but the change from slashes to arguments https://github.com/rails/rails/pull/31530/files was in fact also cosmetic cause we already knew it does not change anything https://twitter.com/tenderlove/status/842064491936280576 馃槃

@rails-bot rails-bot bot added the railties label Sep 11, 2020
@p8
Copy link
Member

p8 commented Sep 29, 2020

This seems like a valid change. I'm also seeing more argument joins in the dummy apps config.

@rails-bot
Copy link

rails-bot bot commented Dec 28, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 28, 2020
@rafaelfranca rafaelfranca merged commit 05e6945 into rails:master Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants