Skip to content

Revert explicit keyword arg brackets

Steve Abrams requested to merge revert-2320 into master

What does this MR do and why?

In !2320 (merged) calls to commit_version_files were updated, wrapping all keyword arguments in brackets {}. This was a mistake as doing so makes them not compatible with Ruby 3.

The only change needed from that MR was the one in lib/release_tools/public_release/cng_image_release.rb where a non-keyword argument was a hash, but it was being implicitly passed without being wrapped in brackets.

In this MR, we revert all changes from !2320 (merged) with the exception of cng_image_release.rb.

This demonstrates what I mean:

[1] pry(main)> Warning[:deprecated] = true
=> true
[2] pry(main)> def foo(a, b, c: 'asdf', d: 'aaa', e: false, f: false)
[2] pry(main)*   p [a,b,c,d,e,f]
[2] pry(main)* end
[4] pry(main)> ASDF = '1'
[5] pry(main)> foo('bar', {ASDF => '1.0'}, c: 'bbb', f: true)
["bar", {"1"=>"1.0"}, "bbb", "aaa", false, true]

# this works as expected and does not need to change, but I changed many to this:

[6] pry(main)> foo('bar', {ASDF => '1.0'}, { c: 'bbb', f: true })
["bar", {"1"=>"1.0"}, "bbb", "aaa", false, true]
2023-04-21 09:51:21.382127 W Ruby -- (pry):8: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
 -- {:source=>"ruby_warnings", :stacktrace=>nil}

# which throws a warning and will not work with Ruby 3, so we revert them in this MR
# The change in cng_image_release.rb was like this:

[9] pry(main)> foo('bar', ASDF => '1.0')
["bar", {"1"=>"1.0"}, "asdf", "aaa", false, false]
2023-04-21 09:52:07.856435 W Ruby -- (pry):12: warning: Passing the keyword argument as the last hash parameter is deprecated
 -- {:source=>"ruby_warnings", :stacktrace=>nil}

# where the param `b` is `ASDF => '1.0'`, so we do need to wrap that one since it's not a keyword argument

[10] pry(main)> foo('bar', {ASDF => '1.0'})
["bar", {"1"=>"1.0"}, "asdf", "aaa", false, false]

Author Check-list

  • [-] Has documentation been updated?

Related to gitlab-com/gl-infra/delivery#2864 (closed)

Edited by Steve Abrams

Merge request reports