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 chewy:journal:clean task for ruby 3.x #874

Merged
merged 5 commits into from
Apr 5, 2023

Conversation

muk-ai
Copy link
Contributor

@muk-ai muk-ai commented Feb 10, 2023

This task seems to fails in ruby 3.x because keyword arguments are not expanded.

$ bundle exec rails "chewy:journal:clean"
rails aborted!
ArgumentError: wrong number of arguments (given 1, expected 0)

Tasks: TOP => chewy:journal:clean
(See full trace by running task with --trace)

ruby: 3.0.4
chewy: 7.2.7

I added double splat operator.

Comment on lines +99 to 102
**[
parse_journal_args(args.extras),
{delete_by_query_options: delete_options}
].reduce({}, :merge)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[1] pry(main)> [
[1] pry(main)*   parse_journal_args(args.extras),
[1] pry(main)*   {delete_by_query_options: delete_options}
[1] pry(main)* ].reduce({}, :merge)
=> {:only=>[], :delete_by_query_options=>{}}

@konalegi konalegi closed this Mar 20, 2023
@konalegi konalegi reopened this Mar 20, 2023
@konalegi
Copy link

@muk-ai could you please provide a matrix for ruby 3.2 https://github.com/toptal/chewy/blob/master/.github/workflows/ruby.yml#L45?

@muk-ai
Copy link
Contributor Author

muk-ai commented Mar 21, 2023

My environment is ruby 3.0.4, I don't think it has anything to do with ruby 3.2.

@konalegi
Copy link

ah, ok, right. Although do you think it's still possible to add kind of spec so your solution is tested on CI through all ruby versions that are defined there?

@muk-ai
Copy link
Contributor Author

muk-ai commented Apr 4, 2023

@konalegi I wasn't sure if I should add the test to this position, but I added it anyway.

@konalegi
Copy link

konalegi commented Apr 4, 2023

@muk-ai thanks for the tests, it looks good, could you please fix rubocop issues?

@muk-ai
Copy link
Contributor Author

muk-ai commented Apr 4, 2023

@konalegi Thanks for your quick review. fixed.

@konalegi
Copy link

konalegi commented Apr 4, 2023

sorry, I've forgot (shame on me), could you please provide an entry in the changelog, for example, https://github.com/toptal/chewy/pull/869/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR7-R9

@muk-ai
Copy link
Contributor Author

muk-ai commented Apr 5, 2023

@konalegi A changelog was added👍

@konalegi konalegi merged commit 8e3fa9a into toptal:master Apr 5, 2023
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.

3 participants