Add Support for Rails 3.1 #99
Conversation
| delegate :configuration, to: EmberCLI | ||
| delegate :tee_path, :npm_path, :bundler_path, to: :configuration | ||
|
|
||
| def non_production? | ||
| !Rails.env.production? |
There was a problem hiding this comment.
Relying on Rails.env.production? is an anti-pattern and totally not gonna work for any production-like environment with a different name. For instance lots of people actually create a separate environment called staging by copying production.rb into staging.rb and deploing it. This change ensures that none of this gonna work anymore. This is the main reason Helpers.non_production? exists in the first place.
There was a problem hiding this comment.
Could we expose these are configurations, with the default value set from the logic in Helpers?
The project that is driving these changes has config.consider_all_requests_local = false
There was a problem hiding this comment.
How do you mean?
config.consider_all_requests_local = false is usually a sign of a production-like environment. Which means that you probably want a production-like behavior from the gem (do nothing, let assets be precompiled).
There was a problem hiding this comment.
Fair enough.
The problem I'm facing is that I've manually set config.use_ember_middleware = true, overriding the value of non_production?. This works for browsing the app, the EmberCLI app's test/index.html is no longer included in the build, since non_production?? "development" : "production" is evaluating to "production".
This is breaking the work done in 0.1.7
There was a problem hiding this comment.
Errm... does it mean that you want ember tests in production?
There was a problem hiding this comment.
Ok, this is a bigger issue that it seems.
The gem has essentially three modes of operation:
-
development
Starts a build server on first request, runs it until the process dies.
-
test
Builds assets once, does nothing after.
-
production
Does nothing.
I guess instead of having this logic being spread out in lots of different places, we could make a single config option to let user set the operation mode manually.
There was a problem hiding this comment.
Also, this discussion doesn't have anything to do with 3.1 support, it's a separate issue, am I wrong?
There was a problem hiding this comment.
@rwz that's probably true.
cc: @rondale-sc
In the mean time, what part of this diff is a point of contention to be being merged?
There was a problem hiding this comment.
Should I reduce the scope of this PR to just the gemspec and inflector changes?
There was a problem hiding this comment.
Yeah, that'd be cool. Let's remove the version changes, and precompile flag from it.
|
@rwz I've responded in #99 (comment) but it's now hidden by an outdated diff |
| @@ -12,6 +12,6 @@ Gem::Specification.new do |spec| | |||
|
|
|||
| spec.required_ruby_version = ">= 1.9.3" | |||
|
|
|||
| spec.add_dependency "railties", "~> 4.0" | |||
| spec.add_dependency "sprockets-rails", "~> 2.0" | |||
| spec.add_dependency "railties", ">= 3.1" | |||
There was a problem hiding this comment.
Let's do "> 3.1", "< 5", since we can't use "~>"
|
So, can you please undo all the changes that are not gemspec and inflection? And then we can merge it. |
|
Oh, and change version comparison to |
* reduce Railties version * only inflect on `CLI` acronym if that API is available
|
@rwz good to go |
|
👍 |
Add Support for Rails 3.1
CLIacronym if that API is available