Replace all instances of is_string with type

[spencertransier]

What does it do?

This PR replaces all instances of the deprecated is_string with the appropriate type in Fastlane ConfigItem's.

According to the Fastlane documentation, type: Boolean shouldn't be used because "Ruby does not have a single class to represent a Boolean type". However, if you search the Fastlane PRs for type: Boolean, there are dozens of examples where type: Boolean was accepted and merged in. I went ahead and used type: Boolean in this PR for consistency and clarity.

A few of these had already been replaced in #425 (search for is_string), but I decided to replicate those changes here since that fork hasn't been merged in yet and it's been nearly 6 months.

Fixes #278

Checklist before requesting a review

• Run bundle exec rubocop to test for code style violations and recommendations
• Add Unit Tests (aka specs/*_spec.rb) if applicable
• Run bundle exec rspec to run the whole test suite and ensure all your tests pass
• Make sure you added an entry in the CHANGELOG.md file to describe your changes under the approprioate existing ### subsection of the existing ## Trunk section.

[AliSoftware]

Nice work and cleanup! ❤️

Left a couple of side-notes for some typos that we could consider fixing in the ConfigItem's descriptions while at it, but otherwise

[oguzkocer]

@spencertransier @AliSoftware Is this the correct type for this item? According to this we are checking if it's an Array or a String. I've also checked the locales parameter we are passing from WPAndroid and Ruby is telling me that it's an Array.

I was trying to test the auto-retry downloading metadata strings PR and I've hit an issue where fastlane is telling me that 'locales' value must be a Hash! Found Array instead. I thought it was something I was doing wrong, but I think this change is the culprit.

[AliSoftware]

Ah, this is confusing indeed!

According to this we are checking if it's an Array or a String.

The params[:locales] is expected to be an Enumerable, on which we call params[:locales].each do |loc| … to iterate over each loc element. And those individual array elements are what we check for them to be an Array or a String.

In practice:

• if params[:locales] is a Hash (e.g. {'fr'=>'fr-FR', 'de'=>'de-DE', 'en'=>'en-US'}, then iterating over that Hash yields elements that each are 2-item arrays of the form [key, value] (e.g. ['fr', 'fr-FR']). We thus then falls into the if loc.is_a?(Array) branch inside the loop.

  irb> {'fr'=>'fr-FR', 'de'=>'de-DE'}.each { |loc| puts loc.inspect }
  ["fr", "fr-FR"]
  ["de", "de-DE"]

• if params[:locales] is an Array, then it can be either an Array of Strings (['fr', 'de', 'en']) and fall into the if loc.is_a?(String) branch inside the loop… or be an Array of 2-element-Arrays ([['fr', 'fr-FR'], ['de', 'de-DE'], ['en', 'en-US']) and fall into the if loc.is_a?(Array) branch.

So in reality, this parameter can be either a Hash or an Array<String> or an Array<[String,String]> (the latter being what WPAndroid uses) 😓

I think down the line our long term plan was to replace this legacy gp_download_metadata with a more modern API, including because of this odd API around the parameters like locales here. But until then we're stuck with this API that is a bit confusing and too flexible to be consistent.

---

I guess the best option here would be to set type: Enumerable and better document the parameter in the description. That being said, I've never seen type: Enumerable being used for a ConfigItem (as it's more common to provide explicit types), so we better test that this more generic type would work and be supported by ConfigItem's implementation. So I guess this would be the ideal time to write some specs for that action (even minimalistic ones for now, that would only focus on feeding different type of inputs for this parameter) to ensure type: Enumerable would work as expected (or what would need to be used instead if not).

[oguzkocer]

Could we temporarily revert the change and open a separate issue for this? Since it's a breaking issue we need to address it immediately and I unfortunately don't have the bandwidth to handle it properly.

I could also test if type: Enumerable works for WPAndroid and handle the hotfix and the releases if that's good enough for now.

cc @spencertransier In case you have more availability to follow up on this better than I can.

[spencertransier]

@oguzkocer I'll revert it and get the 7.1.2 release going.

[spencertransier]

Created separate issue for this here: #484