-
Notifications
You must be signed in to change notification settings - Fork 21.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Added a generator option to remove the public/index.html file when ge…
…nerating a new Rails application The option is: -i, [--skip-index-html] # Skip public/index.html file
- Loading branch information
1 parent
b49a7dd
commit 5c1109a
Showing
3 changed files
with
15 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5c1109a
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.
I've just seen this and I want to suggest something different.
What about reverting this commit, removing index.html and defining a rails engine inside railties that you can mount in '/' if the routes.rb file has still no routes defined ?.
5c1109a
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.
👍 @spastorino
5c1109a
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.
It does not need to be an engine, it can be a simple rack app. And it sounds like a good idea. :)
Checking if a route exists can be cumbersome... so we could simply append the route (at the bottom), then as soon as someone defines a new "/", it won't be matched anymore.
5c1109a
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.
👍 @josevalim
5c1109a
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.
@josevalim true :) and agreed. Not sure about always adding "/" though, because if the user doesn't add a root route it will end having that index view in production.
Anyway we can just generate the "index" route inside config/route.rb itself suggesting the user to just change that.
5c1109a
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.
@spastorino we can add it just in development. We already do it:
https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L25
5c1109a
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.
@spastorino add it only for development environment like /rails/info/properties
5c1109a
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.
Sounds good bros
5c1109a
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.
👍 @steveklabnik @josevalim
5c1109a
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.
👍 @jeremy @spastorino
5c1109a
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.
5c1109a
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.
👍 @josevalim @guilleiguaran @spastorino
5c1109a
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.
👍 @jeremy @josevalim @spastorino @steveklabnik
5c1109a
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.
👍 to the 👍s
Also 👍 to the feature.
5c1109a
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.