Config and Location swift decodable conformance

[Scollaco]

📲 What

Makes the models Config, Location, and Project.Country conform to Swift.Decodable.

🤔 Why

This puts us one step closer to remove the Argo library from our codebase. Argo was included in our project before Swift Decodable be released by Apple and its removal will result in the deletion of tons of boilerplate code. Besides that, all the new models no longer conform to the Argo.Decodable protocol.

🛠 How

To make the models conform to Swift Decodable, I simply created an extension that conforms to the protocol with a init(decoder:) function. The unit tests make sure that the models are being decoded correctly. In the future, we will need to create a new fetch function on our Service+RequestHelpers with the correct return type (SignalProducer<A, ErrorEnvelope>) and maybe with a keyDecodingStrategy.

✅ Acceptance criteria

• Config decoding shouldn't be affected, since we are not directly using Swift decoder yet.

• ConfigTests should pass.

• Location decoding shouldn't be affected.

• LocationTests should pass.

• Project.Country decoding shouldn't be affected.

• Project.CountryTests should pass.

• Got planned refactors to come

• Will add more tests

• Need feedback on a design

[justinswart]

Looking good, had two suggestions 👍

[justinswart]

I think we can replace the if/else here and set the property directly with:

self.stripePublishableKey = try values
  .nestedContainer(keyedBy: StripeCodingKeys.self, forKey: .stripe)
  .decode(String.self, forKey: .stripePublishableKey)

[Scollaco]

Nice solution!

[justinswart]

I think let's put these on new lines for consistency?

[justinswart]

So what if we tried to use these decodings in place of the Argo ones?

[Scollaco]

So what if we tried to use these decodings in place of the Argo ones?

We would just need to create a new request function to return a SignalProducer<M, ErrorEnvelope> where M == Swift.Decodable and use JSONDecoder instead. I actually had to create this function temporarily to see if it was working. So this model would basically work with Argo and Swift.Decodable.

[justinswart]

@Scollaco I think we should merge this into a feature branch because it will be unused code until we have the request functions to handle it, what do you think?

[Scollaco]

@Scollaco I think we should merge this into a feature branch because it will be unused code until we have the request functions to handle it, what do you think?

Yeah, I see your point. The only con I see in waiting is that we would have to do the same to all models (make them conform to both Argo and Swift decodable) before merge the feature branch, and this can take a long time and end up forgotten, like it happened in the past. And since these 2 models still conform Argo.Decodable, our codebase as it is wouldn't be affected, making possible to get away from argo progressively. We can also talk to the team and see what they think.