Adding WatchOS availability & decoding conformance to Card #30
Conversation
|
Hi, thanks for the contribution!
This isn't super surprising to me, I haven't been following MTG (let alone Scryfall's API changes) much for these past few years, I've just been relying on the smoke tests to tell me if something is broken. I'd love to know what cards specifically you were seeing decoding errors with so that I can get them added to the smoke test suite.
I'm not at my laptop right now so I can't check but I'm mostly certain that the
I wouldn't be opposed to adding the tvOS and VisionOS platforms. Feel free to do that in this PR, otherwise there's some work I've been meaning to get a PR open for and I could add those platforms there. |
|
shall we get this merge @JacobHearst |
| public var lang: String | ||
| /// A link to where you can begin paginating all re/prints for this card on Scryfall’s API. | ||
| public var printsSearchUri: String | ||
| public var printsSearchUri: String? |
There was a problem hiding this comment.
Scryfall's API docs don't list this as an optional field, have you seen cards where this property is nil?
| public var relatedUris: [String: String] | ||
| /// This card's release date | ||
| public var releasedAt: String | ||
| public var releasedAt: String? |
There was a problem hiding this comment.
Scryfall's API docs don't list this as an optional, have you seen cards where this property is nil?
| case usd, eur, tix, usdFoil, usdEtched | ||
| } | ||
|
|
||
| public enum SecurityStamp: String, Codable, Sendable { |
There was a problem hiding this comment.
Please add a header doc for this type
3 participants
I noticed while trying to decode a JSON response from Scryfall directly into a Card object that a lot of fields were either not accounted for or weren't listed as optional.
Also, in order to correctly decode the responses, accompanying CodingKeys needed to be defined for fields such as scryfallSetUri ->"scryfall_set_uri"
I know this is a much larger change, so please feel free to ask questions. I'm very open to feedback on this!
(Also the app I'm using to consume this framework has a watchos companion, so I needed to add availability for WatchOS as well. Honestly I see no reason to not just also include tvOS and VisionOS as well)