Lexicon expects data within an inner object with $bytes property#15
Lexicon expects data within an inner object with $bytes property#15
Conversation
|
preemptively tagged this 1.1.11 for downstream while we review this |
ThisIsMissEm
left a comment
ThisIsMissEm
left a comment
There was a problem hiding this comment.
Looks fine to me at a high level, just a few smaller comments.
| keyedBy: LexiconBytes.CodingKeys.self | ||
| ) | ||
| self.bytes = try container.decode(Data.self, forKey: .bytes) | ||
| } catch { |
There was a problem hiding this comment.
should this be catching a specific error here? i.e., I think this would catch both if the container doesn't exist or if the decoding failed, but I think we want to only fall through if the container doesn't exist?
There was a problem hiding this comment.
yea... I think that needs us to move the decode out of the do closure
|
|
||
| let lexiconBytes = currentKey as! [String: Any] | ||
|
|
||
| let base64 = lexiconBytes["$bytes"] as! String |
There was a problem hiding this comment.
Is there meant to be another assertion here?
| let base64 = lexiconBytes["$bytes"] as! String | ||
| } | ||
|
|
||
| @Test func testValidCycle() throws { |
There was a problem hiding this comment.
I'm not sure what this test case and the next are really testing, given they don't have any assertions?
There was a problem hiding this comment.
that decode doesn't throw. a more extensive test is appropriate, certainly
There was a problem hiding this comment.
there's a bunch of pointy (!) and less pointy (try) edges here that were part of turning this around Monday night on short notice.
Will go through and make this a proper test that reports how it fails instead of just failing.
| self.bytes = try container.decode(Data.self, forKey: .bytes) | ||
| } catch { | ||
| let container = try decoder.singleValueContainer() | ||
| self.bytes = try container.decode(Data.self) |
There was a problem hiding this comment.
You may want to set a flag here: "needs upgrade" — though we can infer this from the version on the declaration record.
2 participants
No description provided.