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

Remove Railtie, Use Rails Engine #2162

Merged

Conversation

jherdman
Copy link
Contributor

@jherdman jherdman commented May 2, 2013

ActiveAdmin had both a Railtie and an Engine. This commit consolidates
this duplication to use engines, and moves locale files to their
conventional location for an Engine.

This commit is a step towards resolving #2126.

ActiveAdmin had both a Railtie and an Engine. This commit consolidates
this duplication to use engines, and moves locale files to their
conventional location for an Engine.

This commit is a step towards resolving activeadmin#2126.
@coreyward
Copy link
Contributor

👍

@seanlinsley
Copy link
Contributor

Are there any known side effects to this?

@jherdman
Copy link
Contributor Author

jherdman commented May 3, 2013

None that I know of. An Engine is merely a Railtie with some built in configuration, this just makes ActiveAdmin adhere to that configuration.

@seanlinsley
Copy link
Contributor

Or for that matter, I'm curious if any AA bugs have been caused by us having both a Railtie and an Engine.

@seanlinsley
Copy link
Contributor

Since this removes the I18n.reload! from #2072, I wonder if translations are breaking with this on Heroku

/cc #334, #596, #1999, #2036

@jherdman
Copy link
Contributor Author

jherdman commented May 3, 2013

I'll whip up a sample app and test it tonight on Heroku.

@jherdman
Copy link
Contributor Author

jherdman commented May 3, 2013

Works like a charm!. Use the usual credentials. If someone changes them by the time you get to this, let me know.

@seanlinsley
Copy link
Contributor

Okay, looks good. Merging! 🐙

seanlinsley added a commit that referenced this pull request May 4, 2013
@seanlinsley seanlinsley merged commit ce996c4 into activeadmin:master May 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants