Skip to content

fix: Correct data type of date-formatted strings #137

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

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

yanniks
Copy link
Contributor

@yanniks yanniks commented Jul 21, 2023

Motivation

While OpenAPI date-formatted strings do not contain information about a specific time, Foundation.Date only supports full timestamps. Also, ISO8601DateTranscoder does not support parsing ISO-8601 dates without time information. This commit introduces the expected behavior that date-only strings are not automatically decoded to Foundation.Date objects. This PR fixes issue #136.

Modifications

.date was removed from the case-condition that defined Foundation.Date as the applicable type name.

Result

date-formatted strings are no longer represented as Foundation.Date but as Swift.String objects.

Test Plan

Tests were adapted so that the correct Swift type for date-formatted strings is ensured.

While OpenAPI date-formatted strings do not contain information about a specific time, `Foundation.Date` only supports full timestamps. Also, `ISO8601DateTranscoder` does not support parsing ISO-8601 dates without time information. This commit introduces the expected behavior that date-only strings are not automatically decoded.
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@czechboy0 czechboy0 requested a review from simonjbeaumont July 21, 2023 18:50
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jul 21, 2023
@czechboy0
Copy link
Contributor

Technically, this will change which type we generate and could be source breaking, but the previously generated code never actually worked, so I'm leaning towards not requiring a minor here, WDYT @simonjbeaumont?

@czechboy0
Copy link
Contributor

@swift-server-bot add to allowlist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants