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

Fix GH #5411. When precompiling, params method is undefined. #5525

Merged
merged 1 commit into from
Mar 22, 2012

Conversation

kennyj
Copy link
Contributor

@kennyj kennyj commented Mar 20, 2012

If we execute assets:precompile, params method is undefined.
But ruby can't judge that params is method (or localvariable). Thus we should rescue NameError.

Please see #5411

BTW: NoMethodError's superclass is NameError.

@kennyj
Copy link
Contributor Author

kennyj commented Mar 20, 2012

I send PR to 3-2-stable, because we don't have rails_helper.rb on master.
I think that we should fix rails/sprockets-rails.

/cc @guilleiguaran

@guilleiguaran
Copy link
Member

@kennyj why the test is in asset_debugging_test.rb instead of assets_test.rb? it's failing only when debugging is on?

@kennyj
Copy link
Contributor Author

kennyj commented Mar 22, 2012

@guilleiguaran You're right. I thought the original problem was related to debug, so I wrote in asset_debugging_test.rb. But now I think that this one wasn't related to it. I'll move it to assets_test.rb.

@kennyj
Copy link
Contributor Author

kennyj commented Mar 22, 2012

done.

@guilleiguaran
Copy link
Member

@spastorino @josevalim please review this

@josevalim
Copy link
Contributor

👍 @guilleiguaran can you port to it to sprockets-rails gem as well? thanks.

@kennyj
Copy link
Contributor Author

kennyj commented Mar 22, 2012

I sent a PR to sprockets-rails (for rails master).
rails/sprockets-rails#1

josevalim added a commit that referenced this pull request Mar 22, 2012
Fix GH #5411. When precompiling, params method is undefined.
@josevalim josevalim merged commit b714140 into rails:3-2-stable Mar 22, 2012
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.

None yet

3 participants