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 Active Support deprecations #27035

Merged
merged 18 commits into from Nov 14, 2016

Conversation

Projects
None yet
7 participants
@pixeltrix
Member

pixeltrix commented Nov 13, 2016

This removes the deprecated methods, files and options from Active Support that were explicitly listed for removal from Rails 5.1 with two exceptions:

  1. The qualified constant methods used two singleton methods from ActiveSupport::QualifiedConstUtils - should those be deprecated or just removed straight out since they're not used anywhere else in Rails and it's unlikely they were used in an application.

  2. The string callbacks and returning false are used elsewhere in the Rails test suite so I want to remove them in a separate PR.

Also there are methods with non-specific deprecations like alias_method_chain - is the plan to remove these in 5.1 also?

@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Nov 14, 2016

Member

should those be deprecated or just removed straight out since they're not used anywhere else in Rails and it's unlikely they were used in an application.

I'd remove them right now.

Also there are methods with non-specific deprecations like alias_method_chain - is the plan to remove these in 5.1 also?

Yes. If they were deprecated in 5.0 should be removed in 5.1

Member

rafaelfranca commented Nov 14, 2016

should those be deprecated or just removed straight out since they're not used anywhere else in Rails and it's unlikely they were used in an application.

I'd remove them right now.

Also there are methods with non-specific deprecations like alias_method_chain - is the plan to remove these in 5.1 also?

Yes. If they were deprecated in 5.0 should be removed in 5.1

@kaspth

kaspth approved these changes Nov 14, 2016

@pixeltrix pixeltrix merged commit e491b2c into master Nov 14, 2016

3 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@pixeltrix pixeltrix deleted the remove-active-support-deprecations branch Nov 14, 2016

@aried3r

This comment has been minimized.

Show comment
Hide comment
@aried3r

aried3r Nov 24, 2016

Contributor

Since Rails 5.0.0.1 in some cases did not point to the location of the deprecated code but instead pointed to the code where the deprecation warning was printed, what is the recommended path to find these last deprecations? Use the 5-0-stable branch directly (where I think this has been fixed)? I was hoping there would be a 5.0.1 with this fix before 5.1.

Example:

DEPRECATION WARNING: before_filter is deprecated and will be removed in Rails 5.1. Use before_action instead. (called from block (2 levels) in <module:ClassMethods> at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.0.1/lib/abstract_controller/callbacks.rb:191)

Source

We only recently made the switch to 5.0 and didn't have time to find all deprecations within our dependencies (our own code doesn't have any).

See also #26686 #24861.

Contributor

aried3r commented Nov 24, 2016

Since Rails 5.0.0.1 in some cases did not point to the location of the deprecated code but instead pointed to the code where the deprecation warning was printed, what is the recommended path to find these last deprecations? Use the 5-0-stable branch directly (where I think this has been fixed)? I was hoping there would be a 5.0.1 with this fix before 5.1.

Example:

DEPRECATION WARNING: before_filter is deprecated and will be removed in Rails 5.1. Use before_action instead. (called from block (2 levels) in <module:ClassMethods> at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.0.1/lib/abstract_controller/callbacks.rb:191)

Source

We only recently made the switch to 5.0 and didn't have time to find all deprecations within our dependencies (our own code doesn't have any).

See also #26686 #24861.

@prathamesh-sonpatki

This comment has been minimized.

Show comment
Hide comment
@prathamesh-sonpatki

prathamesh-sonpatki Nov 24, 2016

Member

#26686 commit is present in 5-0-stable branch from which 5.0.1 will be released so with 5.0.1 you will be able to see proper locations of the deprecations. Also it will be released before 5.1.

See 363d558

Member

prathamesh-sonpatki commented Nov 24, 2016

#26686 commit is present in 5-0-stable branch from which 5.0.1 will be released so with 5.0.1 you will be able to see proper locations of the deprecations. Also it will be released before 5.1.

See 363d558

@aried3r

This comment has been minimized.

Show comment
Hide comment
@aried3r

aried3r Nov 24, 2016

Contributor

Great, thank you! I was unsure (and unable to find any information) about how Rails plans to release everything.

Contributor

aried3r commented Nov 24, 2016

Great, thank you! I was unsure (and unable to find any information) about how Rails plans to release everything.

@aried3r

This comment has been minimized.

Show comment
Hide comment
@aried3r

aried3r Dec 7, 2016

Contributor

so with 5.0.1 you will be able to see proper locations of the deprecations

I recently tried 5.0.1.rc1, and while 38 test cases now fail (I do not yet know why) I also don't see the correct location for all deprecations.

Works (deprecation in Turbolinks Classic):

DEPRECATION WARNING: before_filter is deprecated and will be removed in Rails 5.1. Use before_action instead. (called from block (3 levels) in <class:Engine> at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/turbolinks-2.5.3/lib/turbolinks.rb:14)

Doesn't work:

DEPRECATION WARNING: Using positional arguments in functional tests has been deprecated,
in favor of keyword arguments, and will be removed in Rails 5.1.

Deprecated style:
get :show, { id: 1 }, nil, { notice: "This is a flash message" }

New keyword style:
get :show, params: { id: 1 }, flash: { notice: "This is a flash message" },
  session: nil # Can safely be omitted.
 (called from process at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.1.rc1/lib/action_controller/test_case.rb:484)

test_case.rb:484. I can't tell which test that is as I ran the whole suite.

Contributor

aried3r commented Dec 7, 2016

so with 5.0.1 you will be able to see proper locations of the deprecations

I recently tried 5.0.1.rc1, and while 38 test cases now fail (I do not yet know why) I also don't see the correct location for all deprecations.

Works (deprecation in Turbolinks Classic):

DEPRECATION WARNING: before_filter is deprecated and will be removed in Rails 5.1. Use before_action instead. (called from block (3 levels) in <class:Engine> at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/turbolinks-2.5.3/lib/turbolinks.rb:14)

Doesn't work:

DEPRECATION WARNING: Using positional arguments in functional tests has been deprecated,
in favor of keyword arguments, and will be removed in Rails 5.1.

Deprecated style:
get :show, { id: 1 }, nil, { notice: "This is a flash message" }

New keyword style:
get :show, params: { id: 1 }, flash: { notice: "This is a flash message" },
  session: nil # Can safely be omitted.
 (called from process at /usr/local/opt/rbenv/versions/2.3.1/lib/ruby/gems/2.3.0/gems/actionpack-5.0.1.rc1/lib/action_controller/test_case.rb:484)

test_case.rb:484. I can't tell which test that is as I ran the whole suite.

*Andrew White*
* Remove deprecated file `active_support/core_ext/time/marshal.rb`

This comment has been minimized.

@bogdan

bogdan Dec 7, 2016

Contributor

The changelog is confusing here. It is better to say which methods were removed.

@bogdan

bogdan Dec 7, 2016

Contributor

The changelog is confusing here. It is better to say which methods were removed.

This comment has been minimized.

@pixeltrix

pixeltrix Dec 9, 2016

Member

@bogdan not sure what you mean - it was an empty file that was kept around so that it didn't break people's require statements so no methods were removed.

@pixeltrix

pixeltrix Dec 9, 2016

Member

@bogdan not sure what you mean - it was an empty file that was kept around so that it didn't break people's require statements so no methods were removed.

This comment has been minimized.

@bogdan

bogdan Dec 9, 2016

Contributor

I thought that there were some methods that were removed and asked to list those... but it looks like it is not the case. Thanks for explaination

@bogdan

bogdan Dec 9, 2016

Contributor

I thought that there were some methods that were removed and asked to list those... but it looks like it is not the case. Thanks for explaination

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment