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 all code deprecated in Rails 5.2 #34954

Merged
merged 25 commits into from Jan 17, 2019

Conversation

@rafaelfranca
Copy link
Member

rafaelfranca commented Jan 17, 2019

@kaspth I need review on the commits with the title "Remove deprecated config.secret_token" and "
Remove secret_token rack env and cookie upgrade code"

class_attribute :represent_boolean_as_integer, default: false
def self.represent_boolean_as_integer=(value) # :nodoc:
ActiveSupport::Deprecation.warn(
"`.represent_boolean_as_integer=` is deprecated and will be removed in Rails 6.1"

This comment has been minimized.

Copy link
@matthewd

matthewd Jan 17, 2019

Member

Perhaps this should warn if value is true (maybe with a message more like "is now always true, so the setting is deprecated and will be removed"?), but raise if it's false?

@kaspth
kaspth approved these changes Jan 17, 2019
Copy link
Member

kaspth left a comment

Those 2 commits look good to me!

@kaspth
kaspth approved these changes Jan 17, 2019
Copy link
Member

kaspth left a comment

Now I checked the rest of them. Looks good!

else
max_updated_column_timestamp
end
timestamp = max_updated_column_timestamp

This comment has been minimized.

Copy link
@kaspth

kaspth Jan 17, 2019

Member

Should we remove the default argument from max_updated_column_timestamp? It's private and I don't see it used anywhere else.

This comment has been minimized.

Copy link
@kaspth

kaspth Jan 17, 2019

Member

Ah, you did 👍

def max_updated_column_timestamp(timestamp_names = timestamp_attributes_for_update_in_model)
timestamp_names
def max_updated_column_timestamp
timestamp_attributes_for_update_in_model
.map { |attr| self[attr] }
.compact
.map(&:to_time)

This comment has been minimized.

Copy link
@kaspth

kaspth Jan 17, 2019

Member

Could we spare the second loop with this?

timestamp_attributes_for_update_in_model.map { |attr| self[attr]&.to_time }.compact
@rafaelfranca rafaelfranca force-pushed the rm-remove-5.2-deprecations branch 3 times, most recently from 3ce2d5d to 8e3f349 Jan 17, 2019
rafaelfranca added 20 commits Jan 14, 2019
We are past 5.1 and it was not extrated yet, so while we still have
plans they will not be realized on 6.0, so it is better to not set
expectations of which release will exclude it just yet.

[ci skip]
`#success?`, `missing?` and `error?` were deprecated in Rails 5.2 in favor of
`#successful?`, `not_found?` and `server_error?`.
…ragment_cache_key`
… of the class
…alid_alter_table_type?`
…ters
Now that secret_token was removed all this code is now dead.
… class as argument of `run`
@rafaelfranca rafaelfranca force-pushed the rm-remove-5.2-deprecations branch from 8e3f349 to 4d51efe Jan 17, 2019
@rafaelfranca rafaelfranca merged commit e65a3a0 into master Jan 17, 2019
3 checks passed
3 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@rafaelfranca rafaelfranca deleted the rm-remove-5.2-deprecations branch Jan 17, 2019
def deprecate_positional_rack_server_and_rewrite_to_option(original_options)
if using
ActiveSupport::Deprecation.warn(<<~MSG)
Passing the Rack server name as a regular argument is deprecated

This comment has been minimized.

Copy link
@y-yagi

y-yagi Jan 18, 2019

Member

This deprecation added by Rails 6.0. Ref: #32058

This comment has been minimized.

Copy link
@y-yagi

y-yagi Jan 18, 2019

Member

I reverted fa791fb with eb63faa.

This comment has been minimized.

Copy link
@rafaelfranca

rafaelfranca Jan 18, 2019

Author Member

Thank you for catching that.

@bogdanvlviv

This comment has been minimized.

@rafaelfranca

This comment has been minimized.

Copy link
Member Author

rafaelfranca commented Feb 1, 2019

I left them by design.

cheezenaan added a commit to cheezenaan/yaml_vault that referenced this pull request Sep 11, 2019
because config.secret_token has been deprecated in v6.0
ref. rails/rails#34954
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.