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

Lift the deprecation from ArrayNode#each_value #301

Merged

Conversation

koic
Copy link
Member

@koic koic commented Jun 29, 2024

This PR lifts the deprecation from ArrayNode#each_value for the following reasons:

  1. HashNode#each_value and HashNode#each_key are not deprecated.
  1. node.values.each is recommended by Style/HashEachMethods cop to be replaced with each_value instead of values.each. And node.each_value do |value| is clearer in intent than node.children.each do |value|.

Since it is a soft deprecation in documentation only, it has not been added to the changelog.

This PR lifts the deprecation from `ArrayNode#each_value` for the following reasons:

1. `HashNode#each_value` and `HashNode#each_key` are not deprecated.

- `HashNode#each_value` ... https://github.com/rubocop/rubocop-ast/blob/v1.31.3/lib/rubocop/ast/node/hash_node.rb#L76-L89
- `HashNode#each_key` ... https://github.com/rubocop/rubocop-ast/blob/v1.31.3/lib/rubocop/ast/node/hash_node.rb#L52-L65

2. `node.values.each` is recommended by `Style/HashEachMethods` cop to be replaced with `each_value` instead of `values.each`.
And `node.each_value do |value|` is clearer in intent than `node.children.each do |value|`.

Since it is a soft deprecation in documentation only, it has not been added to the changelog.
@marcandre marcandre merged commit d361dcd into rubocop:master Jun 30, 2024
19 checks passed
@marcandre
Copy link
Contributor

You are right, thank you ❤️

@koic koic deleted the lift_the_deprecation_from_array_node_each_value branch June 30, 2024 06:24
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

2 participants