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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate application_record.rb file before model file #25319

Merged

Conversation

morgoth
Copy link
Member

@morgoth morgoth commented Jun 7, 2016

Previously model file was generated first, which resulted in
inheriting from ActiveRecord::Base, but since application_record.rb
is generated as well, it should already be used.

@rails-bot
Copy link

r? @arthurnn

(@rails-bot has picked a reviewer for you, use r? to override)

@morgoth morgoth force-pushed the generate-application-record-first branch from d803ad7 to d627af8 Compare June 7, 2016 20:47
Previously model file was generated first, which resulted in
inheriting from `ActiveRecord::Base`, but since application_record.rb
is generated as well, it should already be used.
@morgoth morgoth force-pushed the generate-application-record-first branch from d627af8 to b991017 Compare June 8, 2016 06:52
@morgoth
Copy link
Member Author

morgoth commented Jun 13, 2016

What do you think about making it into 5.0? It would be a nice improvement for people upgrading.

@rafaelfranca rafaelfranca merged commit a973485 into rails:master Jun 13, 2016
@gsamokovarov
Copy link
Contributor

We already have a PR for that: #24756.

@rafaelfranca
Copy link
Member

I'll apply both

@morgoth
Copy link
Member Author

morgoth commented Jun 13, 2016

Thanks. I didn't notice the first one

rafaelfranca added a commit that referenced this pull request Jun 13, 2016
Generate application_record.rb file before model file
@gsamokovarov
Copy link
Contributor

👍

@morgoth morgoth deleted the generate-application-record-first branch June 13, 2016 15:28
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

6 participants