-
Notifications
You must be signed in to change notification settings - Fork 44
Fix enum associated value decoding #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit 1447ea2.
We had a bug in enum decoding that resulted in the decoder missing a few columns along the way.
| // if true { | ||
| if ProcessInfo.processInfo.environment["SPI_GENERATE_DOCS"] != nil { | ||
| if ProcessInfo.processInfo.environment["SPI_GENERATE_DOCS"] != nil | ||
| || ProcessInfo.processInfo.environment["CI"] != nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's ensure enum tests are actually running in CI.
| let photo = try decoder.decode(Photo.self) | ||
| let note = try decoder.decode(String.self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to call decoder.decode for every type so that the query decoder's cursor advances to the proper column offset.
| fileprivate enum Kind { | ||
| case link(URL) | ||
| case note(String) | ||
| case notes(Attachment.Notes) | ||
| @Column(as: [String].JSONRepresentation.self) | ||
| case notesInline([String]) | ||
| case video(Attachment.Video) | ||
| case image(Attachment.Image) | ||
| } | ||
|
|
||
| @Selection fileprivate struct Notes { | ||
| @Column(as: [String].JSONRepresentation.self) | ||
| let list: [String] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acosmicflamingo Sorry, I reverted these tests since they weren't actually running on CI and were actually failing with the changes I made.
I do wonder if this PR fixes some of the issues you were seeing, though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No big deal! I'll have to see when I'm in front of a computer.
We had a bug in enum decoding that resulted in the decoder missing a few columns along the way.
Fixes #218.