Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
BACKWARDS INCOMPATIBLE: Renamed application.rb to application_control…
…ler.rb and removed all the special casing that was in place to support the former. You must do this rename in your own application when you upgrade to this version [DHH]
- Loading branch information
Showing
7 changed files
with
11 additions
and
16 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
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
File renamed without changes.
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
fcce1f1
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.
Saw that coming ;)
fcce1f1
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.
Makes sense for 3.0, not sure about 2.3 though.
fcce1f1
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.
Maybe still load application.rb if it exists and spit out a deprecation notice? That way there can be a transitioning version.
fcce1f1
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 remember way back when…“ApplicationController” was just called “Application.”
Those were the days.
fcce1f1
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.
+1 ryanb
fcce1f1
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.
great!
fcce1f1
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.
yay!
fcce1f1
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.
This should be a 3.0 change, imo.
fcce1f1
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.
+1 ryanb
phase this change in gradually =)
fcce1f1
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.
Finally!
fcce1f1
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.
that makes more sense.
fcce1f1
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.
+1 for ryanb. Any big api change should at lease get some deprecation warning and isn’t this a little much for a minor release.
fcce1f1
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.
We haven’t really decided if 2.2.next will be 2.3 or 3.0. Also, ‘rake rails:update’ task should/will take care of renaming application.rb → application_controller.rb.
fcce1f1
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.
Isn’t 2.3 going to be released as 3.0? I thought that is how it worked. 1.2.x lead to 2.0.x … so 2.2.x will give way to 3.0.x!
Regardless, keep up the good work!
fcce1f1
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 remember when this was being discussed. Glad to see it finally making its way into core!
fcce1f1
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.
When this gets pushed into a release we’ll naturally take care to make the upgrade path as smooth as possible.
However, if you’re tracking edge right now, just after a branch, you’re braver than I am :)
fcce1f1
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.
At last!
fcce1f1
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.
1+ for ranaming
fcce1f1
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.
Anyone else getting an ActionController::MethodNotAllowed when visiting / since this?
fcce1f1
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.
nm its unrelated, seems map.root is generating me a POST rather than a GET route :S
fcce1f1
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 still think a transitioning version (where application.rb is loaded and deprecated) would be nice. Not for development (it’s easy enough to rename a file), but for deployment in production. Now you’ll have to push this change to the server the exact same time you upgrade Rails. Having a stepping-stone version where both application.rb and application_controller.rb work would ease this transition.
fcce1f1
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.
After thinking about it a bit more I suppose it’s not as big of an issue. It’s easy enough to make a second branch, change the gem version in the config, fix all the problems, and then deploy that all to the server in one go.
fcce1f1
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.
@ryanb: 2.2.something could end up being the transitioning version you’re talking about, or we could add the deprecation / warnings into master.
Again, this isn’t how we’re going to ship anything, and chasing edge is risky, especially this soon after a branch where we have a bunch of features / patches we pushed out as ‘too risky’.
fcce1f1
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.
@topfunky: oh yeah, well I remember abstract_application_controller.rb. Take that!
fcce1f1
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.
abstract_application_controller.rb! whoa
fcce1f1
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 only problem with calling it application_controller is that it indicates that it is a controller in MVC, which it isn’t. It has features that all other controllers share.
fcce1f1
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.
@evilgeenius the class has been called ApplicationController for quite some time. This patch just makes the file name consistent with the class name. Consistency is a good thing.
fcce1f1
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.
huzzah!
fcce1f1
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.
@ryanb i concede
fcce1f1
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.
With this commit you’ll need to update your phusion passenger — but it might be better to hack the solution yourself than try to be fancy and update the gem.
See my note in comments at http://ryandaigle.com/articles/2008/11/19/what-s-new-in-edge-rails-application-rb-duality-is-no-more