sets script_name to always be a string.

[JONBRWN]

see issue: #17615
and commit: 8cbcd19
which states script_name should always return a string
when script_name is nil in the options hash, script_name is set to nil.

options = {script_name: nil}
script_name = options.delete(:script_name) {‘’} # => nil

[egilburg]

I guess it can just be options.delete(:script_name) || '' then?

[JONBRWN]

@egilburg Yeah, that's what I did in my pull request

[egilburg]

@JONBRWN I see the following diff

options.delete(:script_name) { '' } || ''

Whereas I think it can be simplified to:

options.delete(:script_name) || ''

Since you are always assigning '' on nil, whether explicit value for key or due to missing key, you don't need to also use the missing-key default of { '' }

[JONBRWN]

@egilburg you are correct, long day! Will edit the pull request when I get home

[sandipransing]

think of using #fetch instead of #delete

options.fetch(:script_name) { '' }

[jGRUBBS]

@sandipransing #fetch does not achieve the desired result. :script_name should be removed from options and should always return a String and never a nil

@egilburg's solution should be accepted:

options.delete(:script_name) || ''

[jGRUBBS]

@tenderlove what do you think? Seeing as you refactored this method in 8cbcd19

[tenderlove]

Seems like a reasonable change, but let me make sure the tests are OK first.

[arthurnn]

dont we need a regression test for this too?

[JONBRWN]

@tenderlove @arthurnn let me know if there's something I can do on my part with the testing

[rafaelfranca]

@JONBRWN we need a test for sure.

[spastorino]

Closed by 1e5290a