-
-
Notifications
You must be signed in to change notification settings - Fork 909
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 #tables deprecation in Rails 5.0 - Fixes #933 #943
Conversation
@@ -38,7 +38,9 @@ def join_table_option_correct? | |||
end | |||
|
|||
def join_table_exists? | |||
if connection.tables.include?(join_table_name) | |||
if (connection.respond_to?(:data_sources) ? |
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.
Avoid multi-line ?: (the ternary operator); use if
/unless
instead.
Don't use parentheses around the condition of an if
.
Fixes #933 |
@@ -37,8 +37,16 @@ def join_table_option_correct? | |||
end | |||
end | |||
|
|||
def tables_and_views(table_or_view) |
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.
What do you think about having this method return connection.data_sources
or connection.tables
instead of a boolean? Also, let's put this in RailsShim -- we have a collection of methods there that deal with compatibility between Rails versions.
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.
@mcmire Good idea. I will make the changes.
@mcmire Do you think the code is better now? I'm opened to any feedback. |
Sorry for the delay. Yup, I think this is good! Do you mind squashing your commits? I'll merge it in after you do that. |
cedb538
to
9b05c31
Compare
@mcmire Can this get merged? Lot of noise from these messages. Thanks! |
@mcmire can you please merge this? seconding petercm's comment |
It would be nice to merge this to avoid Rails 5 deprecation warnings. |
@mcmire 4thing the above comments |
@mcmire Any chance of getting this merged soon? |
@mikemerritt I'm setting up the Travis CI environment for Rails 5.0 so we can merge it safely. |
Fixes #933 |
… to 5.0.4 and 4.2.9 in Appraisal Add missing json dependency for rdoc in Rails 4.1 and 4.2 failing in TravisCI Merge pull request thoughtbot#1015 from thoughtbot/master 2 months ago Merge pull request thoughtbot#1014 from guialbuk/rails-5-readme Add installation instructions for Rails 5.0 Merge pull request thoughtbot#943 from guialbuk/fix-933 Fix #tables deprecation in Rails 5.0 and keep compatibility with Rails 4.x Merge pull request thoughtbot#1013 from guialbuk/travis-rails-5 Fix multi-line block code style Add jbuilder to rails 4.2 and 5.0 tests Update appraisal to rails 5.0.2 Add rails-controller-testing gem to rails 5.0 tests Fetch pry gem using HTTPS Don't protect attributes under Rails 5 Don't require ActiveResource tests under Rails 5 capture, silence_stream, and silence_stderr were removed in Rails 5 Exclude Ruby 2.3 from Rails 4.0.x jobs in Travis CI Add specific ruby versions so Travis CI downloads the right rvm binaries Exclude Ruby 2.0 and 2.1 from Rails 5.0 jobs in Travis CI Update Rails versions to 5.0.4 and 4.2.9 in Appraisal
…ning See issue here: thoughtbot/shoulda-matchers#933 See fix here: thoughtbot/shoulda-matchers#943 DEPRECATION WARNING: #tables currently returns both tables and views. This behavior is deprecated and will be changed with Rails 5.1 to only return tables. Use #data_sources instead.
…ning See issue here: thoughtbot/shoulda-matchers#933 See fix here: thoughtbot/shoulda-matchers#943 DEPRECATION WARNING: #tables currently returns both tables and views. This behavior is deprecated and will be changed with Rails 5.1 to only return tables. Use #data_sources instead.
And keep compatibility with Rails 4.x